[ovs-dev] [PATCH] flow: Support OF1.5+ (draft) actset_output field.

Jarno Rajahalme jrajahalme at nicira.com
Thu Oct 9 15:46:00 UTC 2014


Small issues with the tests, otherwise:

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>

On Oct 9, 2014, at 8:05 AM, Ben Pfaff <blp at nicira.com> wrote:

> This field allows a flow table to match on the output port currently in the
> action set.
> 
> OF1.3 and OF1.4 should use ONFOXM_ET_ACTSET_OUTPUT; OF1.5+ should use
> OXM_OF_ACTSET_OUTPUT.  The current patch uses the former for all
> versions.
> 

You plan to fix this in another patch later?

(snip)

> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 652a2a3..974720b 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -490,6 +490,83 @@ AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> 
> +dnl Tests that 1.5 set-field with mask in the metadata register.
> +AT_SETUP([ofproto-dpif - masked set-field into metadata])
> +OVS_VSWITCHD_START
> +ADD_OF_PORTS([br0], [1], [2], [3])
> +AT_DATA([flows.txt], [dnl
> +table=0     actions=set_field:0xfafafafa5a5a5a5a->metadata,goto_table(1)
> +table=1     actions=set_field:0x6b/0xff->metadata,goto_table(2)
> +table=2,metadata=0xfafafafa5a5a5a6b  actions=2
> +
> +# These low-priority rules shouldn't match.  They're here only to make really
> +# sure that the test fails if either of the above rules fails to match.
> +table=0,priority=0                        actions=3
> +table=1,priority=0                        actions=3
> +table=2,priority=0                        actions=3
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flows.txt])
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +

Did the above belong to an earlier patch? I have no problem it being added with this patch, unless the test is already in.

> +AT_SETUP([ofproto-dpif - actset_output])
> +OVS_VSWITCHD_START
> +ADD_OF_PORTS([br0], [1], [2], [3], [4], [5], [6], [7], [8], [9], [10], [11])
> +AT_DATA([flows.txt], [dnl
> +table=0     actions=write_actions(output(2)),goto_table(1)
> +table=1     actions=move:ONFOXM_ET_ACTSET_OUTPUT[[0..31]]->OXM_OF_PKT_REG0[[0..31]],goto_table(2)
> +
> +# Verify that actset_output got set.
> +table=2,priority=20,actset_output=2  actions=4,goto_table(3)
> +table=2,priority=10                  actions=5,goto_table(3)
> +
> +# Verify that xreg0 got copied properly from actset_output.
> +table=3,priority=20,xreg0=2  actions=6,goto_table(4)
> +table=3,priority=10          actions=7,goto_table(4)
> +
> +# Verify that adding a group action unsets actset_output.
> +table=4 actions=write_actions(group(5)),goto_table(5)
> +table=5,priority=20,actset_output=unset  actions=8,goto_table(6)
> +table=5,priority=10                      actions=9,goto_table(6)
> +
> +# Verify that adding another output action doesn't change actset_output
> +# (since there's still a group).
> +table=6 actions=write_actions(output(3)),goto_table(7)
> +table=7,priority=20,actset_output=unset  actions=10
> +table=7,priority=10                      actions=11
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flows br0 flows.txt])
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 4,6,8,10
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +
> +dnl Tests that 1.5 set-field with mask in the metadata register.
> +AT_SETUP([ofproto-dpif - masked set-field into metadata])
> +OVS_VSWITCHD_START
> +ADD_OF_PORTS([br0], [1], [2], [3])
> +AT_DATA([flows.txt], [dnl
> +table=0     actions=set_field:0xfafafafa5a5a5a5a->metadata,goto_table(1)
> +table=1     actions=set_field:0x6b/0xff->metadata,goto_table(2)
> +table=2,metadata=0xfafafafa5a5a5a6b  actions=2
> +
> +# These low-priority rules shouldn't match.  They're here only to make really
> +# sure that the test fails if either of the above rules fails to match.
> +table=0,priority=0                        actions=3
> +table=1,priority=0                        actions=3
> +table=2,priority=0                        actions=3
> +])
> +AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flows.txt])
> +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
> +AT_CHECK([tail -1 stdout], [0], [Datapath actions: 2
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> +

This seems to be repeated from above.

> AT_SETUP([ofproto-dpif - push-pop])
> OVS_VSWITCHD_START
> ADD_OF_PORTS([br0], [20], [21], [22], [33], [90])
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 51efd37..9f35d5e 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1177,6 +1177,7 @@ OVS_VSWITCHD_START
>       metadata: arbitrary mask
>       in_port: exact match or wildcard
>       in_port_oxm: exact match or wildcard
> +      actset_output: exact match or wildcard
>       pkt_mark: arbitrary mask
>       reg0: arbitrary mask
>       reg1: arbitrary mask
> @@ -1247,7 +1248,7 @@ AT_CHECK(
> # Check that the configuration was updated.
> mv expout orig-expout
> sed 's/classifier/main/
> -73s/1000000/1024/' < orig-expout > expout
> +74s/1000000/1024/' < orig-expout > expout
> AT_CHECK([ovs-ofctl -O OpenFlow13 dump-table-features br0 | sed '/^$/d
> /^OFPST_TABLE_FEATURES/d'], [0], [expout])
> OVS_VSWITCHD_STOP
> diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
> index bcc3f33..b8d41fb 100644
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -1119,6 +1119,16 @@ system components in order to facilitate interaction between subsystems.
> On Linux this corresponds to the skb mark but the exact implementation is
> platform-dependent.
> .
> +.IP \fBactset_output=\fIport\fR
> +Matches the output port currently in the OpenFlow action set, where
> +\fIport\fR may be an OpenFlow port number or keyword
> +(e.g. \fBLOCAL\fR).  If there is no output port in the OpenFlow action
> +set, or if the output port will be ignored (e.g. because there is an
> +output group in the OpenFlow action set), then the value will be
> +\fBUNSET\fR.
> +.IP
> +This field was introduced in Open vSwitch 2.4 to conform with the
> +OpenFlow 1.5 (draft) specification.
> .PP
> Defining IPv6 flows (those with \fBdl_type\fR equal to 0x86dd) requires
> support for NXM.  The following shorthand notations are available for
> -- 
> 2.1.0
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev




More information about the dev mailing list