On Wed, Oct 29, 2014 at 04:15:51PM -0700, Jarno Rajahalme wrote:
> 
> On Oct 9, 2014, at 11:11 PM, Ben Pfaff <b...@nicira.com> 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 <jrajaha...@nicira.com>
> >> 
> >> Thanks.
> >> 
> >>> On Oct 9, 2014, at 8:05 AM, Ben Pfaff <b...@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?
> >> 
> >> 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?

I was hoping you would take a look at v2.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to