| Message ID | 20220616063247.517147-12-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 |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 16 Jun 2022, at 8:32, Adrian Moreno wrote: > Create a small helper script and check that flows used in ofp-actions.at > are parseable. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- Changes look good to me. Acked-by: Eelco Chaudron <echaudro@redhat.com>
On 6/16/22 08:32, Adrian Moreno wrote: > Create a small helper script and check that flows used in ofp-actions.at > are parseable. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > tests/automake.mk | 2 ++ > tests/ofp-actions.at | 18 +++++++++++++++++ > tests/ovs-test-ofparse.py | 42 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 62 insertions(+) > create mode 100755 tests/ovs-test-ofparse.py > > diff --git a/tests/automake.mk b/tests/automake.mk > index 261fbb942..ab23f04ca 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -19,6 +19,7 @@ EXTRA_DIST += \ > $(OVSDB_CLUSTER_TESTSUITE) \ > tests/atlocal.in \ > $(srcdir)/package.m4 \ > + $(srcdir)/tests/ovs-test-ofparse.py \ > $(srcdir)/tests/testsuite \ > $(srcdir)/tests/testsuite.patch > > @@ -525,6 +526,7 @@ CHECK_PYFILES = \ > tests/flowgen.py \ > tests/mfex_fuzzy.py \ > tests/ovsdb-monitor-sort.py \ > + tests/ovs-test-ofparse.py \ Should this be called just test-ofparse.py for consistency? Same for the other script. > tests/test-daemon.py \ > tests/test-json.py \ > tests/test-jsonrpc.py \ > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at > index 9d820eba6..c75f3ccb5 100644 > --- a/tests/ofp-actions.at > +++ b/tests/ofp-actions.at > @@ -329,6 +329,7 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) This should be '$PYTHON3 $srcdir/ovs-test-ofparse.py'. The same for other places and the other patch. > AT_CLEANUP > > AT_SETUP([OpenFlow 1.0 "instruction" translations]) > @@ -359,6 +360,7 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-instructions OpenFlow10 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) > AT_CLEANUP > > AT_SETUP([OpenFlow 1.1 action translation]) > @@ -502,6 +504,7 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow11 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) > AT_CLEANUP > > AT_SETUP([OpenFlow 1.1 instruction translation]) > @@ -737,6 +740,7 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow12 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) > AT_CLEANUP > > dnl Our primary goal here is to verify OpenFlow 1.3-specific changes, > @@ -798,6 +802,7 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow13 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) > AT_CLEANUP > > dnl Our primary goal here is to verify that OpenFlow 1.5-specific changes, > @@ -827,17 +832,20 @@ AT_CAPTURE_FILE([experr]) > AT_CHECK( > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow15 < input.txt], > [0], [expout], [experr]) > +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) > AT_CLEANUP > > AT_SETUP([ofp-actions - inconsistent MPLS actions]) > OVS_VSWITCHD_START > dnl OK: Use fin_timeout action on TCP flow > AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)']) > +AT_CHECK([echo 'tcp actions=fin_timeout(idle_timeout=1)' | ovs-test-ofparse.py]) > dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS > AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'], > [1], [], [dnl > ovs-ofctl: none of the usable flow formats (OpenFlow10,NXM) is among the allowed flow formats (OpenFlow11) > ]) > +AT_CHECK([echo 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)' | ovs-test-ofparse.py]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > @@ -853,6 +861,8 @@ AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl > NXST_FLOW reply: > mpls actions=load:0xa->OXM_OF_MPLS_LABEL[[]] > ]) > +AT_CHECK([echo 'mpls actions=set_field:10->mpls_label' | ovs-test-ofparse.py]) > +AT_CHECK([echo 'mpls actions=load:0xa->OXM_OF_MPLS_LABEL[[]]'| ovs-test-ofparse.py]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > @@ -862,14 +872,17 @@ OVS_VSWITCHD_START > dnl OpenFlow 1.0 has an "enqueue" action. For OpenFlow 1.1+, we translate > dnl it to a series of actions that accomplish the same thing. > AT_CHECK([ovs-ofctl -O OpenFlow10 add-flow br0 'actions=enqueue(123,456)']) > +AT_CHECK([echo 'actions=enqueue(123,456)' | ovs-test-ofparse.py]) > AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl > NXST_FLOW reply: > actions=enqueue:123:456 > ]) > +AT_CHECK([echo 'actions=enqueue:123:456' | ovs-test-ofparse.py]) > AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip], [0], [dnl > OFPST_FLOW reply (OF1.3): > reset_counts actions=set_queue:456,output:123,pop_queue > ]) > +AT_CHECK([echo 'actions=set_queue:456,output:123,pop_queue' | ovs-test-ofparse.py]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > @@ -887,6 +900,8 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl > OFPST_FLOW reply (OF1.1): > ip actions=mod_nw_ttl:123 > ]) > +AT_CHECK([echo 'ip,actions=mod_nw_ttl:123' | ovs-test-ofparse.py]) > +AT_CHECK([echo 'ip actions=load:0x7b->NXM_NX_IP_TTL[[]]' | ovs-test-ofparse.py]) > OVS_VSWITCHD_STOP > AT_CLEANUP > > @@ -898,10 +913,12 @@ dnl OpenFlow 1.1, but no other version, has a "mod_nw_ecn" action. > dnl Check that we translate it properly for OF1.0 and OF1.2. > dnl (OF1.3+ should be the same as OF1.2.) > AT_CHECK([ovs-ofctl -O OpenFlow11 add-flow br0 'ip,actions=mod_nw_ecn:2']) > +AT_CHECK([echo 'ip,actions=mod_nw_ecn:2' | ovs-test-ofparse.py]) > AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl > NXST_FLOW reply: > ip actions=load:0x2->NXM_NX_IP_ECN[[]] > ]) > +AT_CHECK([echo 'ip actions=load:0x2->NXM_NX_IP_ECN[[]]' | ovs-test-ofparse.py]) > AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl > OFPST_FLOW reply (OF1.1): > ip actions=mod_nw_ecn:2 > @@ -910,6 +927,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow12 dump-flows br0 | ofctl_strip], [0], [dnl > OFPST_FLOW reply (OF1.2): > ip actions=set_field:2->nw_ecn > ]) > +AT_CHECK([echo 'ip actions=set_field:2->nw_ecn' | ovs-test-ofparse.py]) > > dnl Check that OF1.2+ set_field to set ECN is translated into the OF1.1 > dnl mod_nw_ecn action. > diff --git a/tests/ovs-test-ofparse.py b/tests/ovs-test-ofparse.py > new file mode 100755 > index 000000000..0a3d3e681 > --- /dev/null > +++ b/tests/ovs-test-ofparse.py > @@ -0,0 +1,42 @@ > +#!/usr/bin/env python3 > +# Copyright (c) 2021 Red Hat, Inc. 2022 ? :) > +# > +# 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. > + > +"""ovs-test-ofparse reads flows from stdin and tries to parse them using > +the python flow parsing library. > +""" > + > +import fileinput > +import sys > + > +from ovs.flow.ofp import OFPFlow > + > + > +def main(): > + for flow in fileinput.input(): > + try: > + result_flow = OFPFlow(flow) > + if flow != str(result_flow): > + print("in: {}".format(flow)) > + print("out: {}".format(str(result_flow))) > + raise ValueError("Flow conversion back to string failed") > + except Exception as e: > + print(e) > + return 1 > + > + return 0 > + > + > +if __name__ == "__main__": > + sys.exit(main())
diff --git a/tests/automake.mk b/tests/automake.mk index 261fbb942..ab23f04ca 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -19,6 +19,7 @@ EXTRA_DIST += \ $(OVSDB_CLUSTER_TESTSUITE) \ tests/atlocal.in \ $(srcdir)/package.m4 \ + $(srcdir)/tests/ovs-test-ofparse.py \ $(srcdir)/tests/testsuite \ $(srcdir)/tests/testsuite.patch @@ -525,6 +526,7 @@ CHECK_PYFILES = \ tests/flowgen.py \ tests/mfex_fuzzy.py \ tests/ovsdb-monitor-sort.py \ + tests/ovs-test-ofparse.py \ tests/test-daemon.py \ tests/test-json.py \ tests/test-jsonrpc.py \ diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index 9d820eba6..c75f3ccb5 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -329,6 +329,7 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow10 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP AT_SETUP([OpenFlow 1.0 "instruction" translations]) @@ -359,6 +360,7 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-instructions OpenFlow10 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP AT_SETUP([OpenFlow 1.1 action translation]) @@ -502,6 +504,7 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow11 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP AT_SETUP([OpenFlow 1.1 instruction translation]) @@ -737,6 +740,7 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow12 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP dnl Our primary goal here is to verify OpenFlow 1.3-specific changes, @@ -798,6 +802,7 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow13 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP dnl Our primary goal here is to verify that OpenFlow 1.5-specific changes, @@ -827,17 +832,20 @@ AT_CAPTURE_FILE([experr]) AT_CHECK( [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-actions OpenFlow15 < input.txt], [0], [expout], [experr]) +AT_CHECK([cat expout | grep 'actions=' | ovs-test-ofparse.py]) AT_CLEANUP AT_SETUP([ofp-actions - inconsistent MPLS actions]) OVS_VSWITCHD_START dnl OK: Use fin_timeout action on TCP flow AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)']) +AT_CHECK([echo 'tcp actions=fin_timeout(idle_timeout=1)' | ovs-test-ofparse.py]) dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'], [1], [], [dnl ovs-ofctl: none of the usable flow formats (OpenFlow10,NXM) is among the allowed flow formats (OpenFlow11) ]) +AT_CHECK([echo 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)' | ovs-test-ofparse.py]) OVS_VSWITCHD_STOP AT_CLEANUP @@ -853,6 +861,8 @@ AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl NXST_FLOW reply: mpls actions=load:0xa->OXM_OF_MPLS_LABEL[[]] ]) +AT_CHECK([echo 'mpls actions=set_field:10->mpls_label' | ovs-test-ofparse.py]) +AT_CHECK([echo 'mpls actions=load:0xa->OXM_OF_MPLS_LABEL[[]]'| ovs-test-ofparse.py]) OVS_VSWITCHD_STOP AT_CLEANUP @@ -862,14 +872,17 @@ OVS_VSWITCHD_START dnl OpenFlow 1.0 has an "enqueue" action. For OpenFlow 1.1+, we translate dnl it to a series of actions that accomplish the same thing. AT_CHECK([ovs-ofctl -O OpenFlow10 add-flow br0 'actions=enqueue(123,456)']) +AT_CHECK([echo 'actions=enqueue(123,456)' | ovs-test-ofparse.py]) AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl NXST_FLOW reply: actions=enqueue:123:456 ]) +AT_CHECK([echo 'actions=enqueue:123:456' | ovs-test-ofparse.py]) AT_CHECK([ovs-ofctl -O OpenFlow13 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.3): reset_counts actions=set_queue:456,output:123,pop_queue ]) +AT_CHECK([echo 'actions=set_queue:456,output:123,pop_queue' | ovs-test-ofparse.py]) OVS_VSWITCHD_STOP AT_CLEANUP @@ -887,6 +900,8 @@ AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.1): ip actions=mod_nw_ttl:123 ]) +AT_CHECK([echo 'ip,actions=mod_nw_ttl:123' | ovs-test-ofparse.py]) +AT_CHECK([echo 'ip actions=load:0x7b->NXM_NX_IP_TTL[[]]' | ovs-test-ofparse.py]) OVS_VSWITCHD_STOP AT_CLEANUP @@ -898,10 +913,12 @@ dnl OpenFlow 1.1, but no other version, has a "mod_nw_ecn" action. dnl Check that we translate it properly for OF1.0 and OF1.2. dnl (OF1.3+ should be the same as OF1.2.) AT_CHECK([ovs-ofctl -O OpenFlow11 add-flow br0 'ip,actions=mod_nw_ecn:2']) +AT_CHECK([echo 'ip,actions=mod_nw_ecn:2' | ovs-test-ofparse.py]) AT_CHECK([ovs-ofctl -O OpenFlow10 dump-flows br0 | ofctl_strip], [0], [dnl NXST_FLOW reply: ip actions=load:0x2->NXM_NX_IP_ECN[[]] ]) +AT_CHECK([echo 'ip actions=load:0x2->NXM_NX_IP_ECN[[]]' | ovs-test-ofparse.py]) AT_CHECK([ovs-ofctl -O OpenFlow11 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.1): ip actions=mod_nw_ecn:2 @@ -910,6 +927,7 @@ AT_CHECK([ovs-ofctl -O OpenFlow12 dump-flows br0 | ofctl_strip], [0], [dnl OFPST_FLOW reply (OF1.2): ip actions=set_field:2->nw_ecn ]) +AT_CHECK([echo 'ip actions=set_field:2->nw_ecn' | ovs-test-ofparse.py]) dnl Check that OF1.2+ set_field to set ECN is translated into the OF1.1 dnl mod_nw_ecn action. diff --git a/tests/ovs-test-ofparse.py b/tests/ovs-test-ofparse.py new file mode 100755 index 000000000..0a3d3e681 --- /dev/null +++ b/tests/ovs-test-ofparse.py @@ -0,0 +1,42 @@ +#!/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. + +"""ovs-test-ofparse reads flows from stdin and tries to parse them using +the python flow parsing library. +""" + +import fileinput +import sys + +from ovs.flow.ofp import OFPFlow + + +def main(): + for flow in fileinput.input(): + try: + result_flow = OFPFlow(flow) + if flow != str(result_flow): + print("in: {}".format(flow)) + print("out: {}".format(str(result_flow))) + raise ValueError("Flow conversion back to string failed") + except Exception as e: + print(e) + return 1 + + return 0 + + +if __name__ == "__main__": + sys.exit(main())
Create a small helper script and check that flows used in ofp-actions.at are parseable. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- tests/automake.mk | 2 ++ tests/ofp-actions.at | 18 +++++++++++++++++ tests/ovs-test-ofparse.py | 42 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+) create mode 100755 tests/ovs-test-ofparse.py