On 14 March 2016 at 14:36, Ben Pfaff <[email protected]> wrote: > On Mon, Mar 14, 2016 at 01:57:57PM -0700, Jarno Rajahalme wrote: > > > > > On Mar 14, 2016, at 10:42 AM, Ben Pfaff <[email protected]> wrote: > > > > > > On Sun, Feb 28, 2016 at 10:33:17PM -0800, Gurucharan Shetty wrote: > > >> From: Jarno Rajahalme <[email protected]> > > >> > > >> Signed-off-by: Jarno Rajahalme <[email protected]> > > > > > > The core of the change here seems to be: > > > > > >> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c > > >> index becf02d..8f6e02e 100644 > > >> --- a/lib/ofp-actions.c > > >> +++ b/lib/ofp-actions.c > > >> @@ -5913,7 +5913,8 @@ ofpacts_execute_action_set(struct ofpbuf > *action_list, > > >> * not be sent anywhere. */ > > >> if (!ofpacts_copy_last(action_list, action_set, OFPACT_GROUP) && > > >> !ofpacts_copy_last(action_list, action_set, OFPACT_OUTPUT) && > > >> - !ofpacts_copy_last(action_list, action_set, > OFPACT_RESUBMIT)) { > > >> + !ofpacts_copy_last(action_list, action_set, OFPACT_RESUBMIT) > && > > >> + !ofpacts_copy_last(action_list, action_set, OFPACT_CT)) { > > >> ofpbuf_clear(action_list); > > >> } > > >> } > > > > > > The change to ofproto-dpif-xlate seems undesirable, because if I read > it > > > correctly it means that every packet translation that involves a group > > > bucket will put that into the log, without even rate-limiting. I think > > > that this change was probably meant for debugging, without meaning to > be > > > applied permanently. > > > > > > > Right, I sent my unpolished patch to Guru, and did not remember about > > the debugging I added to ofproto-dpif-xlate. So, to be clear, the > > ofproto-dpif-xlate changes in this patch should not be applied, > > otherwise this should be good to go. > > OK. If the xlate changes are dropped, then: > Acked-by: Ben Pfaff <[email protected]> >
It took a while to decide whether to merge this patch. Back then, we were considering dp_hash based approach for group selection and it was not very clear to me whether this patch would still be relevant. But, based on Jarno's RFC patch "xlate: Use dp_hash for select groups", this is still relevant. I intend to drop all the changes in ofproto-dpif-xlate as requested in the review and apply this with the following commit message after some time (I have also modified the output formatting of the unit test to get consistent results). Author: Jarno Rajahalme <[email protected]> Date: Tue May 24 00:25:36 2016 -0700 ofp-actions: Allow conntrack action in group buckets. Conntrack action used in group buckets lets us do simple load-balancing. Signed-off-by: Jarno Rajahalme <[email protected]> [[email protected] updated the commit message and made a small change to the test output format] Signed-off-by: Gurucharan Shetty <[email protected]> _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
