Re: [ovs-dev] [PATCH v1 12/18] tests: Wrap test-odp to also run python parsers

Adrian Moreno Thu, 13 Jan 2022 07:32:16 -0800


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 <[email protected]>
---
  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 asense out of the string. empty info or match does not yield a malformed flowerror. 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 amethod 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


--
Adrián Moreno

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 12/18] tests: Wrap test-odp to also run python parsers

Reply via email to