python: Return list of actions for odp action clone.

Sometimes we don't want to return the result of a nested key-value
decoding as a dictionary but as a list of dictionaries. This happens
when we parse actions where keys can be repeated.

Refactor code that already takes that into account from ofp_act.py to
kv.py and use it for datapath action "clone".

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
Acked-by: Mike Pattrick <mkp@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Signed-off-by: Simon Horman <horms@ovn.org>

python/ovs/flow/kv.py

@@ -320,7 +320,26 @@ def decode_nested_kv(decoders, value):
     return {kv.key: kv.value for kv in parser.kv()}
 
 
-def nested_kv_decoder(decoders=None):
+def decode_nested_kv_list(decoders, value):
+    """A key-value decoder that extracts nested key-value pairs and returns
+    them in a list of dictionary.
+
+    Args:
+        decoders (KVDecoders): The KVDecoders to use.
+        value (str): The value string to decode.
+    """
+    if not value:
+        # Mark as flag
+        return True
+
+    parser = KVParser(value, decoders)
+    parser.parse()
+    return [{kv.key: kv.value} for kv in parser.kv()]
+
+
+def nested_kv_decoder(decoders=None, is_list=False):
     """Helper function that creates a nested kv decoder with given
     KVDecoders."""
+    if is_list:
+        return functools.partial(decode_nested_kv_list, decoders)
     return functools.partial(decode_nested_kv, decoders)

python/ovs/flow/odp.py

@@ -337,7 +337,8 @@ class ODPFlow(Flow):
         }
 
         _decoders["clone"] = nested_kv_decoder(
-            KVDecoders(decoders=_decoders, default_free=decode_free_output)
+            KVDecoders(decoders=_decoders, default_free=decode_free_output),
+            is_list=True,
         )
 
         return {
@@ -350,7 +351,8 @@ class ODPFlow(Flow):
                             KVDecoders(
                                 decoders=_decoders,
                                 default_free=decode_free_output,
-                            )
+                            ),
+                            is_list=True,
                         ),
                     }
                 )

python/ovs/flow/ofp.py

@@ -31,7 +31,6 @@ from ovs.flow.ofp_act import (
     decode_dec_ttl,
     decode_chk_pkt_larger,
     decode_zone,
-    decode_exec,
     decode_learn,
 )
 
@@ -336,8 +335,7 @@ class OFPFlow(Flow):
                         "table": decode_int,
                         "nat": decode_nat,
                         "force": decode_flag,
-                        "exec": functools.partial(
+                        "exec": nested_kv_decoder(
-                            decode_exec,
                             KVDecoders(
                                 {
                                     **OFPFlow._encap_actions_decoders_args(),
@@ -345,6 +343,7 @@ class OFPFlow(Flow):
                                     **OFPFlow._meta_action_decoders_args(),
                                 }
                             ),
+                            is_list=True,
                         ),
                         "alg": decode_default,
                     }
@@ -359,6 +358,7 @@ class OFPFlow(Flow):
                     }
                 )
             ),
+            # learn moved to _clone actions.
         }
 
     @staticmethod
@@ -400,11 +400,11 @@ class OFPFlow(Flow):
         """
         return {
             "learn": decode_learn(action_decoders),
-            "clone": functools.partial(
+            "clone": nested_kv_decoder(
-                decode_exec, KVDecoders(action_decoders)
+                KVDecoders(action_decoders), is_list=True
             ),
-            "write_actions": functools.partial(
+            "write_actions": nested_kv_decoder(
-                decode_exec, KVDecoders(action_decoders)
+                KVDecoders(action_decoders), is_list=True
             ),
         }
 

python/ovs/flow/ofp_act.py

@@ -1,8 +1,5 @@
 """Defines decoders for OpenFlow actions.
 """
-
-import functools
-
 from ovs.flow.decoders import (
     decode_default,
     decode_time,
@@ -258,19 +255,6 @@ def decode_zone(value):
     return decode_field(value)
 
 
-def decode_exec(action_decoders, value):
-    """Decodes the value of the 'exec' keyword (part of the ct action).
-
-    Args:
-        decode_actions (KVDecoders): The decoders to be used to decode the
-            nested exec.
-        value (string): The string to be decoded.
-    """
-    exec_parser = KVParser(value, action_decoders)
-    exec_parser.parse()
-    return [{kv.key: kv.value} for kv in exec_parser.kv()]
-
-
 def decode_learn(action_decoders):
     """Create the decoder to be used to decode the 'learn' action.
 
@@ -338,4 +322,4 @@ def decode_learn(action_decoders):
         default_free=learn_field_decoding_free,
     )
 
-    return functools.partial(decode_exec, learn_decoder)
+    return nested_kv_decoder(learn_decoder, is_list=True)

python/ovs/tests/test_odp.py

@@ -453,21 +453,37 @@ def test_odp_fields(input_string, expected):
             ],
         ),
         (
-            "actions:clone(1)" ",clone(clone(push_vlan(vid=12,pcp=0),2),1)",
+            "actions:clone(1),clone(clone(push_vlan(vid=12,pcp=0),2),1)",
             [
-                KeyValue("clone", {"output": {"port": 1}}),
+                KeyValue("clone", [{"output": {"port": 1}}]),
                 KeyValue(
                     "clone",
-                    {
+                    [
-                        "output": {"port": 1},
+                        {
-                        "clone": {
+                            "clone": [
-                            "push_vlan": {
+                                {
-                                "vid": 12,
+                                    "push_vlan": {
-                                "pcp": 0,
+                                        "vid": 12,
-                            },
+                                        "pcp": 0,
-                            "output": {"port": 2},
+                                    },
+                                },
+                                {"output": {"port": 2}},
+                            ]
                         },
-                    },
+                        {"output": {"port": 1}},
+                    ],
+                ),
+            ],
+        ),
+        (
+            "actions:clone(recirc(0x1),recirc(0x2))",
+            [
+                KeyValue(
+                    "clone",
+                    [
+                        {"recirc": 1},
+                        {"recirc": 2},
+                    ],
                 ),
             ],
         ),