| Message ID | 20211122112256.2011194-13-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | python: add flow parsing library | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
Some small remarks. On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > test-odp is used to parse datapath flow actions and matches within the > odp tests. Wrap calls to this tool in a python script that also parses > them using the python flow parsing library. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > tests/automake.mk | 3 +- > tests/odp.at | 36 ++++++++--------- > tests/ovs-test-dpparse.py | 83 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 103 insertions(+), 19 deletions(-) > create mode 100755 tests/ovs-test-dpparse.py > > diff --git a/tests/automake.mk b/tests/automake.mk > index 5dc805ef4..be69f3d16 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -21,7 +21,8 @@ EXTRA_DIST += \ > $(srcdir)/package.m4 \ > $(srcdir)/tests/testsuite \ > $(srcdir)/tests/testsuite.patch \ > - $(srcdir)/tests/ovs-test-ofparse.py > + $(srcdir)/tests/ovs-test-ofparse.py \ > + $(srcdir)/tests/ovs-test-dpparse.py Add in alphabetical order And maybe also add it to CHECK_PYFILES to capture flake8 errors. > > COMMON_MACROS_AT = \ > diff --git a/tests/odp.at b/tests/odp.at > index 07a5cfe39..69e86c5c1 100644 > --- a/tests/odp.at > +++ b/tests/odp.at > @@ -105,7 +105,7 @@ sed -i'back' 's/\(skb_mark(0)\),\(ct\)/\1,ct_state(0),ct_zone(0),\2/' odp-out.tx > sed -i'back' 's/\(skb_mark([[^)]]*)\),\(recirc\)/\1,ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),\2/' odp-out.txt > sed -i'back' 's/\(in_port(1)\),\(eth\)/\1,packet_type(ns=0,id=0),\2/' odp-out.txt > > -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` > ]) > AT_CLEANUP > > @@ -192,7 +192,7 @@ sed -n 's/,frag=no),.*/,frag=later)/p' odp-base.txt > sed 's/^/skb_priority(0),tunnel(tun_id=0xfedcba9876543210,src=10.0.0.1,dst=10.0.0.2,ttl=128,erspan(ver=2,dir=1,hwid=0x7/0xf),flags(df|key)),skb_mark(0),recirc_id(0),dp_hash(0),/' odp-base.txt > ) > odp.txt > AT_CAPTURE_FILE([odp.txt]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` > ]) > AT_CLEANUP > > @@ -239,25 +239,25 @@ AT_DATA([odp-tcp6.txt], [dnl > in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1/::255,dst=::2/::255,label=0/0xf0,proto=10/0xf0,tclass=0x70/0xf0,hlimit=128/0xf0,frag=no) > in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1,dst=::2,label=0,proto=6,tclass=0,hlimit=128,frag=no),tcp(src=80/0xff00,dst=8080/0xff) > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` > ]) > AT_CLEANUP > > @@ -385,14 +385,14 @@ check_pkt_len(size=200,gt(ct(nat)),le(drop)) > check_pkt_len(size=200,gt(set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))),le(set(eth(src=00:01:02:03:04:06,dst=10:11:12:13:14:16)))) > lb_output(1) > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], > [`cat actions.txt` > ]) > AT_CLEANUP > > AT_SETUP([OVS datapath actions parsing and formatting - invalid forms]) > dnl This caused a hang in older versions. > -AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions > +AT_CHECK([echo 'encap_nsh@:{@' | ovs-test-dpparse.py ovstest test-odp parse-actions > ], [0], [dnl > odp_actions_from_string: error > ]) > @@ -427,7 +427,7 @@ data_invalid=$(printf '%*s' 131018 | tr ' ' "a") > echo "userspace(pid=1234567,userdata(${data_valid}),tunnel_out_port=10)" >> actions.txt > echo "userspace(pid=1234567,userdata(${data_invalid}),tunnel_out_port=10)" >> actions.txt > > -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl > `cat actions.txt | head -1` > odp_actions_from_string: error > `cat actions.txt | head -3 | tail -1` > @@ -443,7 +443,7 @@ actions=$(printf 'set(encap()),%.0s' $(seq 8190)) > echo "${actions}set(encap())" > actions.txt > echo "${actions}set(encap()),set(encap())" >> actions.txt > > -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl > `cat actions.txt | head -1` > odp_actions_from_string: error > ]) > @@ -457,7 +457,7 @@ dnl sequence of keys. 'syntax error' indicates oversized list of keys. > keys=$(printf 'encap(),%.0s' $(seq 16382)) > echo "${keys}encap()" > keys.txt > echo "${keys}encap(),encap()" >> keys.txt > -AT_CHECK([ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl > +AT_CHECK([ovs-test-dpparse.py ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl > odp_flow_key_to_flow: error (duplicate encap attribute in flow key; the flow key in error is: encap()) > odp_flow_from_string: error (syntax error at encap()) > ]) > @@ -467,7 +467,7 @@ AT_SETUP([OVS datapath keys parsing and formatting - 33 nested encap ]) > AT_DATA([odp-in.txt], [dnl > encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap())))))))))))))))))))))))))))))))) > ]) > -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [dnl > +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [dnl > odp_flow_from_string: error (syntax error at encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))) > ]) > AT_CLEANUP > diff --git a/tests/ovs-test-dpparse.py b/tests/ovs-test-dpparse.py > new file mode 100755 > index 000000000..455b6ea22 > --- /dev/null > +++ b/tests/ovs-test-dpparse.py > @@ -0,0 +1,83 @@ > +#!/usr/bin/env python3 > +# Copyright (c) 2021 Red Hat, 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. > + > +# Breaks lines read from stdin into groups using blank lines as > +# group separators, then sorts lines within the groups for > +# reproducibility. Left over? > +# ovs-test-ofparse is just a wrapper around ovs-ofctl Cut/paste without updating? ovs-test-dpparse and ovstest test-odp > +# that also runs the python flow parsing utility to check that flows are > +# parseable Add dot. > + > +import subprocess > +import sys > +import re > + > +from ovs.flows.odp import ODPFlowFactory > + > +diff_regexp = re.compile(r"\d{2}: (\d{2}|\(none\)) -> (\d{2}|\(none\))$") > + > + > +def run(input_data): > + p = subprocess.Popen( > + sys.argv[1:], > + stdin=subprocess.PIPE, > + stdout=subprocess.PIPE, > + stderr=subprocess.PIPE, > + ) > + out, err = p.communicate(input_data.encode("utf-8")) > + > + print(out.decode("utf-8"), file=sys.stdout, end="") > + print(err.decode("utf-8"), file=sys.stderr, end="") > + return p.returncode, out, err > + > + > +def main(): > + return_code = 0 > + input_data = sys.stdin.read() > + return_code, out, err = run(input_data) > + > + if return_code == 0: > + flows = list() > + for line in input_data.split("\n"): > + if not ( > + "error" in line # skip errors > + or line.strip() == "" # skip empty lines > + or line.strip()[0] == "#" # skip comments > + ): > + flows.append(line) > + > + factory = ODPFlowFactory() > + for flow in flows: > + if any( > + c in sys.argv > + for c in ["parse-keys", "parse-wc-keys", "parse-filter"] > + ): > + # Add actions=drop so that the flow is properly formatted > + flow += " actions:drop" > + elif "parse-actions" in sys.argv: > + flow = "actions:" + flow I’m wondering why “malformed odp flow: %s" % odp_string” is not generated as you are missing the matching part? Guess it’s getting late already ;) > + try: > + _ = factory.from_string(flow) Should the _ here not be the same as flow? Guess checking this should be a good test to see if the conversion is good both ways? I hacked this for this patch, and it seem to work: try: result_flow = factory.from_string(flow) if flow != str(result_flow): print("in : {}".format(flow)) print("out: {}".format(result_flow)) raise ValueError("Flow conversion back to string failed!") Same for the previous patch? > + except Exception as e: > + print(e) > + return 1 > + > + return return_code > + > + > +if __name__ == "__main__": > + sys.exit(main()) > -- > 2.31.1
On 12/23/21 15:43, Eelco Chaudron wrote: > Some small remarks. > > On 22 Nov 2021, at 12:22, Adrian Moreno wrote: > >> test-odp is used to parse datapath flow actions and matches within the >> odp tests. Wrap calls to this tool in a python script that also parses >> them using the python flow parsing library. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> tests/automake.mk | 3 +- >> tests/odp.at | 36 ++++++++--------- >> tests/ovs-test-dpparse.py | 83 +++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 103 insertions(+), 19 deletions(-) >> create mode 100755 tests/ovs-test-dpparse.py >> >> diff --git a/tests/automake.mk b/tests/automake.mk >> index 5dc805ef4..be69f3d16 100644 >> --- a/tests/automake.mk >> +++ b/tests/automake.mk >> @@ -21,7 +21,8 @@ EXTRA_DIST += \ >> $(srcdir)/package.m4 \ >> $(srcdir)/tests/testsuite \ >> $(srcdir)/tests/testsuite.patch \ >> - $(srcdir)/tests/ovs-test-ofparse.py >> + $(srcdir)/tests/ovs-test-ofparse.py \ >> + $(srcdir)/tests/ovs-test-dpparse.py > > Add in alphabetical order > > And maybe also add it to CHECK_PYFILES to capture flake8 errors. > >> >> COMMON_MACROS_AT = \ >> diff --git a/tests/odp.at b/tests/odp.at >> index 07a5cfe39..69e86c5c1 100644 >> --- a/tests/odp.at >> +++ b/tests/odp.at >> @@ -105,7 +105,7 @@ sed -i'back' 's/\(skb_mark(0)\),\(ct\)/\1,ct_state(0),ct_zone(0),\2/' odp-out.tx >> sed -i'back' 's/\(skb_mark([[^)]]*)\),\(recirc\)/\1,ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),\2/' odp-out.txt >> sed -i'back' 's/\(in_port(1)\),\(eth\)/\1,packet_type(ns=0,id=0),\2/' odp-out.txt >> >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` >> ]) >> AT_CLEANUP >> >> @@ -192,7 +192,7 @@ sed -n 's/,frag=no),.*/,frag=later)/p' odp-base.txt >> sed 's/^/skb_priority(0),tunnel(tun_id=0xfedcba9876543210,src=10.0.0.1,dst=10.0.0.2,ttl=128,erspan(ver=2,dir=1,hwid=0x7/0xf),flags(df|key)),skb_mark(0),recirc_id(0),dp_hash(0),/' odp-base.txt >> ) > odp.txt >> AT_CAPTURE_FILE([odp.txt]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` >> ]) >> AT_CLEANUP >> >> @@ -239,25 +239,25 @@ AT_DATA([odp-tcp6.txt], [dnl >> in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1/::255,dst=::2/::255,label=0/0xf0,proto=10/0xf0,tclass=0x70/0xf0,hlimit=128/0xf0,frag=no) >> in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1,dst=::2,label=0,proto=6,tclass=0,hlimit=128,frag=no),tcp(src=80/0xff00,dst=8080/0xff) >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` >> ]) >> AT_CLEANUP >> >> @@ -385,14 +385,14 @@ check_pkt_len(size=200,gt(ct(nat)),le(drop)) >> check_pkt_len(size=200,gt(set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))),le(set(eth(src=00:01:02:03:04:06,dst=10:11:12:13:14:16)))) >> lb_output(1) >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], >> [`cat actions.txt` >> ]) >> AT_CLEANUP >> >> AT_SETUP([OVS datapath actions parsing and formatting - invalid forms]) >> dnl This caused a hang in older versions. >> -AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions >> +AT_CHECK([echo 'encap_nsh@:{@' | ovs-test-dpparse.py ovstest test-odp parse-actions >> ], [0], [dnl >> odp_actions_from_string: error >> ]) >> @@ -427,7 +427,7 @@ data_invalid=$(printf '%*s' 131018 | tr ' ' "a") >> echo "userspace(pid=1234567,userdata(${data_valid}),tunnel_out_port=10)" >> actions.txt >> echo "userspace(pid=1234567,userdata(${data_invalid}),tunnel_out_port=10)" >> actions.txt >> >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl >> `cat actions.txt | head -1` >> odp_actions_from_string: error >> `cat actions.txt | head -3 | tail -1` >> @@ -443,7 +443,7 @@ actions=$(printf 'set(encap()),%.0s' $(seq 8190)) >> echo "${actions}set(encap())" > actions.txt >> echo "${actions}set(encap()),set(encap())" >> actions.txt >> >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl >> `cat actions.txt | head -1` >> odp_actions_from_string: error >> ]) >> @@ -457,7 +457,7 @@ dnl sequence of keys. 'syntax error' indicates oversized list of keys. >> keys=$(printf 'encap(),%.0s' $(seq 16382)) >> echo "${keys}encap()" > keys.txt >> echo "${keys}encap(),encap()" >> keys.txt >> -AT_CHECK([ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl >> +AT_CHECK([ovs-test-dpparse.py ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl >> odp_flow_key_to_flow: error (duplicate encap attribute in flow key; the flow key in error is: encap()) >> odp_flow_from_string: error (syntax error at encap()) >> ]) >> @@ -467,7 +467,7 @@ AT_SETUP([OVS datapath keys parsing and formatting - 33 nested encap ]) >> AT_DATA([odp-in.txt], [dnl >> encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap())))))))))))))))))))))))))))))))) >> ]) >> -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [dnl >> +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [dnl >> odp_flow_from_string: error (syntax error at encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))) >> ]) >> AT_CLEANUP >> diff --git a/tests/ovs-test-dpparse.py b/tests/ovs-test-dpparse.py >> new file mode 100755 >> index 000000000..455b6ea22 >> --- /dev/null >> +++ b/tests/ovs-test-dpparse.py >> @@ -0,0 +1,83 @@ >> +#!/usr/bin/env python3 >> +# Copyright (c) 2021 Red Hat, 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. >> + >> +# Breaks lines read from stdin into groups using blank lines as >> +# group separators, then sorts lines within the groups for >> +# reproducibility. > > Left over? > Yep. >> +# ovs-test-ofparse is just a wrapper around ovs-ofctl > > Cut/paste without updating? ovs-test-dpparse and ovstest test-odp > >> +# that also runs the python flow parsing utility to check that flows are >> +# parseable > > Add dot. > >> + >> +import subprocess >> +import sys >> +import re >> + >> +from ovs.flows.odp import ODPFlowFactory >> + >> +diff_regexp = re.compile(r"\d{2}: (\d{2}|\(none\)) -> (\d{2}|\(none\))$") >> + >> + >> +def run(input_data): >> + p = subprocess.Popen( >> + sys.argv[1:], >> + stdin=subprocess.PIPE, >> + stdout=subprocess.PIPE, >> + stderr=subprocess.PIPE, >> + ) >> + out, err = p.communicate(input_data.encode("utf-8")) >> + >> + print(out.decode("utf-8"), file=sys.stdout, end="") >> + print(err.decode("utf-8"), file=sys.stderr, end="") >> + return p.returncode, out, err >> + >> + >> +def main(): >> + return_code = 0 >> + input_data = sys.stdin.read() >> + return_code, out, err = run(input_data) >> + >> + if return_code == 0: >> + flows = list() >> + for line in input_data.split("\n"): >> + if not ( >> + "error" in line # skip errors >> + or line.strip() == "" # skip empty lines >> + or line.strip()[0] == "#" # skip comments >> + ): >> + flows.append(line) >> + >> + factory = ODPFlowFactory() >> + for flow in flows: >> + if any( >> + c in sys.argv >> + for c in ["parse-keys", "parse-wc-keys", "parse-filter"] >> + ): >> + # Add actions=drop so that the flow is properly formatted >> + flow += " actions:drop" >> + elif "parse-actions" in sys.argv: >> + flow = "actions:" + flow > > I’m wondering why “malformed odp flow: %s" % odp_string” is not generated as you are missing the matching part? > Guess it’s getting late already ;) > Right now, we only require "actions:" to be present in order to actually make a sense out of the string. empty info or match does not yield a malformed flow error. But sure, I guess we can make it fail. I'll also change the unit tests. >> + try: >> + _ = factory.from_string(flow) > > Should the _ here not be the same as flow? Guess checking this should be a good test to see if the conversion is good both ways? > I hacked this for this patch, and it seem to work: > > try: > result_flow = factory.from_string(flow) > if flow != str(result_flow): > print("in : {}".format(flow)) > print("out: {}".format(result_flow)) > raise ValueError("Flow conversion back to string failed!") > > Same for the previous patch? > Good idea. The str(flow) just yields the original flow. So I think we need a method that actually prints the KeyValue pairs? >> + except Exception as e: >> + print(e) >> + return 1 >> + >> + return return_code >> + >> + >> +if __name__ == "__main__": >> + sys.exit(main()) >> -- >> 2.31.1 >
diff --git a/tests/automake.mk b/tests/automake.mk index 5dc805ef4..be69f3d16 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -21,7 +21,8 @@ EXTRA_DIST += \ $(srcdir)/package.m4 \ $(srcdir)/tests/testsuite \ $(srcdir)/tests/testsuite.patch \ - $(srcdir)/tests/ovs-test-ofparse.py + $(srcdir)/tests/ovs-test-ofparse.py \ + $(srcdir)/tests/ovs-test-dpparse.py COMMON_MACROS_AT = \ diff --git a/tests/odp.at b/tests/odp.at index 07a5cfe39..69e86c5c1 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -105,7 +105,7 @@ sed -i'back' 's/\(skb_mark(0)\),\(ct\)/\1,ct_state(0),ct_zone(0),\2/' odp-out.tx sed -i'back' 's/\(skb_mark([[^)]]*)\),\(recirc\)/\1,ct_state(0),ct_zone(0),ct_mark(0),ct_label(0),\2/' odp-out.txt sed -i'back' 's/\(in_port(1)\),\(eth\)/\1,packet_type(ns=0,id=0),\2/' odp-out.txt -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [`cat odp-out.txt` ]) AT_CLEANUP @@ -192,7 +192,7 @@ sed -n 's/,frag=no),.*/,frag=later)/p' odp-base.txt sed 's/^/skb_priority(0),tunnel(tun_id=0xfedcba9876543210,src=10.0.0.1,dst=10.0.0.2,ttl=128,erspan(ver=2,dir=1,hwid=0x7/0xf),flags(df|key)),skb_mark(0),recirc_id(0),dp_hash(0),/' odp-base.txt ) > odp.txt AT_CAPTURE_FILE([odp.txt]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-wc-keys < odp.txt], [0], [`cat odp.txt` ]) AT_CLEANUP @@ -239,25 +239,25 @@ AT_DATA([odp-tcp6.txt], [dnl in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1/::255,dst=::2/::255,label=0/0xf0,proto=10/0xf0,tclass=0x70/0xf0,hlimit=128/0xf0,frag=no) in_port(1),eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15),eth_type(0x86dd),ipv6(src=::1,dst=::2,label=0,proto=6,tclass=0,hlimit=128,frag=no),tcp(src=80/0xff00,dst=8080/0xff) ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x1235' < odp-base.txt], [0], [`cat odp-eth-type.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_vlan=99,ip' < odp-vlan-base.txt], [0], [`cat odp-vlan.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='ip,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='dl_type=0x0800,nw_src=35.8.2.199,nw_dst=172.16.0.199' < odp-base.txt], [0], [`cat odp-ipv4.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='icmp,nw_src=35.8.2.199' < odp-base.txt], [0], [`cat odp-icmp.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='arp,arp_spa=1.2.3.5' < odp-base.txt], [0], [`cat odp-arp.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp.txt` ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-filter filter='tcp6,tp_src=90' < odp-base.txt], [0], [`cat odp-tcp6.txt` ]) AT_CLEANUP @@ -385,14 +385,14 @@ check_pkt_len(size=200,gt(ct(nat)),le(drop)) check_pkt_len(size=200,gt(set(eth(src=00:01:02:03:04:05,dst=10:11:12:13:14:15))),le(set(eth(src=00:01:02:03:04:06,dst=10:11:12:13:14:16)))) lb_output(1) ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [`cat actions.txt` ]) AT_CLEANUP AT_SETUP([OVS datapath actions parsing and formatting - invalid forms]) dnl This caused a hang in older versions. -AT_CHECK([echo 'encap_nsh@:{@' | ovstest test-odp parse-actions +AT_CHECK([echo 'encap_nsh@:{@' | ovs-test-dpparse.py ovstest test-odp parse-actions ], [0], [dnl odp_actions_from_string: error ]) @@ -427,7 +427,7 @@ data_invalid=$(printf '%*s' 131018 | tr ' ' "a") echo "userspace(pid=1234567,userdata(${data_valid}),tunnel_out_port=10)" >> actions.txt echo "userspace(pid=1234567,userdata(${data_invalid}),tunnel_out_port=10)" >> actions.txt -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl `cat actions.txt | head -1` odp_actions_from_string: error `cat actions.txt | head -3 | tail -1` @@ -443,7 +443,7 @@ actions=$(printf 'set(encap()),%.0s' $(seq 8190)) echo "${actions}set(encap())" > actions.txt echo "${actions}set(encap()),set(encap())" >> actions.txt -AT_CHECK_UNQUOTED([ovstest test-odp parse-actions < actions.txt], [0], [dnl +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-actions < actions.txt], [0], [dnl `cat actions.txt | head -1` odp_actions_from_string: error ]) @@ -457,7 +457,7 @@ dnl sequence of keys. 'syntax error' indicates oversized list of keys. keys=$(printf 'encap(),%.0s' $(seq 16382)) echo "${keys}encap()" > keys.txt echo "${keys}encap(),encap()" >> keys.txt -AT_CHECK([ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl +AT_CHECK([ovs-test-dpparse.py ovstest test-odp parse-keys < keys.txt | sed 's/encap(),//g'], [0], [dnl odp_flow_key_to_flow: error (duplicate encap attribute in flow key; the flow key in error is: encap()) odp_flow_from_string: error (syntax error at encap()) ]) @@ -467,7 +467,7 @@ AT_SETUP([OVS datapath keys parsing and formatting - 33 nested encap ]) AT_DATA([odp-in.txt], [dnl encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap())))))))))))))))))))))))))))))))) ]) -AT_CHECK_UNQUOTED([ovstest test-odp parse-keys < odp-in.txt], [0], [dnl +AT_CHECK_UNQUOTED([ovs-test-dpparse.py ovstest test-odp parse-keys < odp-in.txt], [0], [dnl odp_flow_from_string: error (syntax error at encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap(encap()))))))))))))))))))))))))))))))))) ]) AT_CLEANUP diff --git a/tests/ovs-test-dpparse.py b/tests/ovs-test-dpparse.py new file mode 100755 index 000000000..455b6ea22 --- /dev/null +++ b/tests/ovs-test-dpparse.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +# Copyright (c) 2021 Red Hat, 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. + +# Breaks lines read from stdin into groups using blank lines as +# group separators, then sorts lines within the groups for +# reproducibility. + + +# ovs-test-ofparse is just a wrapper around ovs-ofctl +# that also runs the python flow parsing utility to check that flows are +# parseable + +import subprocess +import sys +import re + +from ovs.flows.odp import ODPFlowFactory + +diff_regexp = re.compile(r"\d{2}: (\d{2}|\(none\)) -> (\d{2}|\(none\))$") + + +def run(input_data): + p = subprocess.Popen( + sys.argv[1:], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + out, err = p.communicate(input_data.encode("utf-8")) + + print(out.decode("utf-8"), file=sys.stdout, end="") + print(err.decode("utf-8"), file=sys.stderr, end="") + return p.returncode, out, err + + +def main(): + return_code = 0 + input_data = sys.stdin.read() + return_code, out, err = run(input_data) + + if return_code == 0: + flows = list() + for line in input_data.split("\n"): + if not ( + "error" in line # skip errors + or line.strip() == "" # skip empty lines + or line.strip()[0] == "#" # skip comments + ): + flows.append(line) + + factory = ODPFlowFactory() + for flow in flows: + if any( + c in sys.argv + for c in ["parse-keys", "parse-wc-keys", "parse-filter"] + ): + # Add actions=drop so that the flow is properly formatted + flow += " actions:drop" + elif "parse-actions" in sys.argv: + flow = "actions:" + flow + try: + _ = factory.from_string(flow) + except Exception as e: + print(e) + return 1 + + return return_code + + +if __name__ == "__main__": + sys.exit(main())
test-odp is used to parse datapath flow actions and matches within the odp tests. Wrap calls to this tool in a python script that also parses them using the python flow parsing library. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- tests/automake.mk | 3 +- tests/odp.at | 36 ++++++++--------- tests/ovs-test-dpparse.py | 83 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 19 deletions(-) create mode 100755 tests/ovs-test-dpparse.py