[ovs-dev] [PATCH 2/4] ofproto: Translate NXAST_SAMPLE_IPFIX actions into SAMPLE dp actions

Ben Pfaff blp at nicira.com
Fri Jan 4 18:13:47 UTC 2013


On Tue, Dec 18, 2012 at 11:52:10AM -0800, Romain Lenglet wrote:
> Signed-off-by: Romain Lenglet <rlenglet at vmware.com>

...

> +/* Compose SAMPLE action for sFlow or IPFIX. */

It would probably be better to document the usage of parameters here, at
the top of the function, rather than with the comment
> +     /* Number of packets out of UINT_MAX to sample. */
later on in the middle of the function.  (I realize you're just moving
existing code around, but we might as well improve it as we go.)

Also, s/UINT_MAX/UINT32_MAX/ since this is specifically 32-bit.

> @@ -5841,6 +5879,21 @@ xlate_fin_timeout(struct action_xlate_ctx *ctx,
>      }
>  }
>  
> +static void
> +xlate_sample_ipfix_action(struct action_xlate_ctx *ctx,
> +                          const struct ofpact_sample_ipfix *osi)
> +{
> +    if (osi->probability > 0) {
> +        union user_action_cookie cookie;
> +        /* Scale the probability from 16-bit to 32-bit while
> +         * representing the exact same percentage. */
> +        uint32_t probability = osi->probability << 16 | osi->probability;

I'd find it reassuring to add ():
    uint32_t probability = (osi->probability << 16) | osi->probability;

> +        compose_ipfix_cookie(osi->probability, osi->obs_point_id, &cookie);
> +        compose_sample_action(ctx->ofproto, ctx->odp_actions, &ctx->flow,
> +                              probability, &cookie);
> +    }
> +}

Again, this patch leaves the implementation incomplete, so it might
ultimately be better to squash it with other patches.



More information about the dev mailing list