Looks good to me. -- Romain Lenglet
----- Original Message ----- > From: "Ben Pfaff" <b...@nicira.com> > To: dev@openvswitch.org > Cc: "Ben Pfaff" <b...@nicira.com> > Sent: Monday, May 6, 2013 3:38:58 PM > Subject: [ovs-dev] [PATCH 1/2] ofproto-dpif: Avoid figuring out sFlow and > IPFIX actions twice. > > Not only is it easier to re-use the actions we already have, this avoids > potential problems due to the state that add_sflow_action() and > add_ipfix_action() look at having possibly been changed by > do_xlate_actions(). Currently those functions appear to look only at > the flow's 'in_port', which currently can't change. However, an upcoming > commit will make it possible for actions to change the flow's 'in_port', > and in addition, with this change, one doesn't have to wonder whether these > functions might look at other state that translation might change. > > Signed-off-by: Ben Pfaff <b...@nicira.com> > --- > ofproto/ofproto-dpif.c | 6 +++--- > 1 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 89a4668..d7d21fa 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -7137,6 +7137,7 @@ xlate_actions(struct action_xlate_ctx *ctx, > } else { > static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1); > struct initial_vals initial_vals; > + size_t sample_actions_len; > uint32_t local_odp_port; > > initial_vals.vlan_tci = ctx->base_flow.vlan_tci; > @@ -7144,6 +7145,7 @@ xlate_actions(struct action_xlate_ctx *ctx, > > add_sflow_action(ctx); > add_ipfix_action(ctx); > + sample_actions_len = ctx->odp_actions->size; > > if (tunnel_ecn_ok(ctx) && (!in_port || may_receive(in_port, ctx))) { > do_xlate_actions(ofpacts, ofpacts_len, ctx); > @@ -7151,9 +7153,7 @@ xlate_actions(struct action_xlate_ctx *ctx, > /* We've let OFPP_NORMAL and the learning action look at the > * packet, so drop it now if forwarding is disabled. */ > if (in_port && !stp_forward_in_state(in_port->stp_state)) { > - ofpbuf_clear(ctx->odp_actions); > - add_sflow_action(ctx); > - add_ipfix_action(ctx); > + ctx->odp_actions->size = sample_actions_len; > } > } > > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev