diff mbox

[ovs-dev,v3,05/16] ofp-actions: Add clone action.

Message ID CALDO+SZFKbD-sP_HZ0oxGG9TfO3T1Rk+WYDhTrqs2K_98VT0kA@mail.gmail.com
State Not Applicable
Headers show

Commit Message

William Tu Dec. 21, 2016, 3:45 p.m. UTC 
Hi Joe,

I think we're missing the normal action for handling the non-IP
traffic. Can you try this patch on top of the clone?


Thanks
William

On Tue, Dec 20, 2016 at 4:39 PM, William Tu <u9012063@gmail.com> wrote:
> Hi Joe,
> Thanks I will take a look.
> William
>
> On Tue, Dec 20, 2016 at 10:25 AM, Joe Stringer <joe@ovn.org> wrote:
>> On 18 December 2016 at 00:18, Ben Pfaff <blp@ovn.org> wrote:
>>> From: William Tu <u9012063@gmail.com>
>>>
>>> This patch adds OpenFlow clone action with syntax as below:
>>> "clone([action][,action...])".  The clone() action makes a copy of the
>>> current packet and executes the list of actions against the packet,
>>> without affecting the packet after the "clone(...)" action.  In other
>>> word, the packet before the clone() and after the clone() is the same,
>>> no matter what actions executed inside the clone().
>>>
>>> Use case 1:
>>> Set different fields and output to different ports without unset
>>> actions=
>>>   clone(mod_dl_src:<mac1>, output:1), clone(mod_dl_dst:<mac2>, output:2), output:3
>>> Since each clone() has independent packet, output:1 has only dl_src modified,
>>> output:2 has only dl_dst modified, output:3 has original packet.
>>>
>>> Similar to case1
>>> actions=
>>>   push_vlan(...), output:2, pop_vlan, push_vlan(...), output:3
>>> can be changed to
>>> actions=
>>>   clone(push_vlan(...), output:2),clone(push_vlan(...), output:3)
>>> without having to add pop_vlan.
>>>
>>> case 2: resubmit to another table without worrying packet being modified
>>>   actions=clone(resubmit(1,2)), ...
>>>
>>> Signed-off-by: William Tu <u9012063@gmail.com>
>>> [blp@ovn.org revised this to omit the "sample" action]
>>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>>
>> It looks like the newly added system-traffic tests are failing on all
>> platforms with this patch applied.. William, will you take a look?

Comments

Joe Stringer Dec. 21, 2016, 6:21 p.m. UTC | #1 
On 21 December 2016 at 07:45, William Tu <u9012063@gmail.com> wrote:
> Hi Joe,
>
> I think we're missing the normal action for handling the non-IP
> traffic. Can you try this patch on top of the clone?
>
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -340,6 +340,7 @@ AT_CLEANUP
>  AT_SETUP([datapath - clone action])
>  OVS_TRAFFIC_VSWITCHD_START()
>
> +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
>  ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)
>
>  ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")

If you force all traffic to be handled with this flow, then the test
will pass but it will not be testing anything.

Looking at the test, I don't really follow how it is supposed to be
guaranteeing that the expected actions are executed on both copies of
the packet. Could you explain it?
diff mbox

Patch

--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -340,6 +340,7 @@  AT_CLEANUP
 AT_SETUP([datapath - clone action])
 OVS_TRAFFIC_VSWITCHD_START()

+AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
 ADD_NAMESPACES(at_ns0, at_ns1, at_ns2)

 ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")