[ovs-dev] [PATCH v2 03/11] ofp-actions: Add clone action.

Ben Pfaff blp at ovn.org
Sun Dec 18 08:01:05 UTC 2016


On Sat, Dec 17, 2016 at 08:19:34AM -0800, William Tu wrote:
> Thanks for the patch, I have some comments.
> 
> > +static char * OVS_WARN_UNUSED_RESULT
> > +parse_CLONE(char *arg, struct ofpbuf *ofpacts,
> > +             enum ofputil_protocol *usable_protocols)
> > +{
> > +
> > +    const size_t ct_offset = ofpacts_pull(ofpacts);
> I will s/ct_offset/clone_offset/g
> 
> > +    struct ofpact_nest *clone = ofpact_put_CLONE(ofpacts);
> > +    char *error;
> > +
> > +    ofpbuf_pull(ofpacts, sizeof *clone);
> > +    error = ofpacts_parse_copy(arg, ofpacts, usable_protocols,
> > +                               false, OFPACT_CLONE);
> should we set to "true" to allow instructions?
> 
> > @@ -7279,6 +7363,10 @@ ofpacts_verify_nested(const struct ofpact *a, enum ofpact_type outer_action)
> >                  return OFPERR_OFPBAC_BAD_ARGUMENT;
> >              }
> >          }
> > +        if (outer_action == OFPACT_CLONE) {
> > +                /* add some constraints. */
> > +                return 0;
> > +        }
> I will remove the above, since we no longer use sample.
> 
> > --- a/tests/ofproto-dpif.at
> > +++ b/tests/ofproto-dpif.at
> > @@ -6431,6 +6431,25 @@ AT_CHECK([ovs-vsctl destroy Flow_Sample_Collector_Set 1], [0], [ignore])
> >  OVS_VSWITCHD_STOP
> >  AT_CLEANUP
> >
> > +dnl OpenFlow Clone action using Sample datapath action
> I will remove "Sample"
> 
> > +AT_SETUP([ofproto-dpif - clone action])
> > +OVS_VSWITCHD_START
> > +add_of_ports br0 1 2 3 4
> > +
> > +AT_DATA([flows.txt], [dnl
> > +in_port=1, actions=clone(set_field:192.168.3.3->ip_src),clone(set_field:192.168.4.4->ip_dst,output:2),clone(mod_dl_src:80:81:81:81:81:81,set_field:192.168.5.5->ip_dst,output:3),output:4
> > +])
> > +AT_CHECK([ovs-ofctl add-flows br0 flows.txt], [0], [ignore])
> > +
> > +AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
> > +
> > +AT_CHECK([tail -1 stdout], [0], [dnl
> > +Datapath actions: set(ipv4(src=10.10.10.2,dst=192.168.4.4)),2,set(eth(src=80:81:81:81:81:81)),set(ipv4(src=10.10.10.2,dst=192.168.5.5)),3,set(eth(src=50:54:00:00:00:09)),set(ipv4(src=10.10.10.2,dst=10.10.10.1)),4
> > +])
> LGTM
> 
> Thanks for the patch. I will submit v3.

Thanks for the comments.  No need to submit a v3, I'll apply these
directly to the version in my series.


More information about the dev mailing list