[ovs-dev] [PATCH 1/2] ofproto-dpif: Avoid figuring out sFlow and IPFIX actions twice.
Romain Lenglet
rlenglet at vmware.com
Mon May 6 22:43:02 UTC 2013
Looks good to me.
--
Romain Lenglet
----- Original Message -----
> From: "Ben Pfaff" <blp at nicira.com>
> To: dev at openvswitch.org
> Cc: "Ben Pfaff" <blp at 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 <blp at 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 at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list