On Oct 9, 2014, at 11:11 PM, Ben Pfaff <[email protected]> wrote: > On Thu, Oct 09, 2014 at 09:16:24AM -0700, Ben Pfaff wrote: >> On Thu, Oct 09, 2014 at 08:46:00AM -0700, Jarno Rajahalme wrote: >>> Small issues with the tests, otherwise: >>> >>> Acked-by: Jarno Rajahalme <[email protected]> >> >> Thanks. >> >>> On Oct 9, 2014, at 8:05 AM, Ben Pfaff <[email protected]> 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? >> >> I think doing it properly requires a generalization and update of the >> nx-match layer. I do plan to do that. I forgot about it. >> >> I'm tempted to say, "I'll commit this now and do that later," but I >> don't think that's really acceptable. I'll do it now and post a v2. >> >>>> 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. >> >> This comes from a patch by Jean Tourrilhes that added a test for >> actset_output. I completely rewrote the actset_output test in that >> patch and forgot about the other tests. I will remove the additional >> tests, because at this point they have been taken from Jean without >> crediting him. (Maybe I should add them back in a separate patch, since >> a Signed-off-by was given, but it's not acceptable and not intentional >> to take them without credit.) > > I posted a v2: > http://openvswitch.org/pipermail/dev/2014-October/047145.html > Despite your kind "ack", I'll wait for further acknowledgment before > applying it.
Just to be clear: Are you waiting for someone else to review the v2, or should I review it? Jarno _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
