[ovs-dev] [mirror 13/13] ofproto-dpif: Get rid of "struct dst".

Justin Pettit jpettit at nicira.com
Thu Nov 17 07:51:25 UTC 2011


On Oct 26, 2011, at 10:09 AM, Ben Pfaff wrote:

> static bool
> is_admissible(struct ofproto_dpif *ofproto, const struct flow *flow,
> -              bool have_packet,
> -              tag_type *tags, int *vlanp, struct ofbundle **in_bundlep)
> +              struct ofport_dpif *in_port, uint16_t vlan, bool warn,
> +              tag_type *tags)

The comment describing this function needs to be updated, since the argument have changed substantially.

> +    /* Check other admissibility requirements. */
> +    if (!is_admissible(ctx->ofproto, &ctx->flow, in_port, vlan,
> +                       ctx->packet != NULL, &ctx->tags)) {
> +        output_mirrors(ctx, vlan, in_bundle, 0);

Do we always want to mirror packets that were determined to be not admissible?  For example, should we be mirroring frames that came in on a destination mirror bundle?

> +    if (in_bundle != out_bundle) {
> +        compose_dsts(ctx, vlan, in_bundle, out_bundle);

I assume you want to store the result in "dst_mirrors", since it's otherwise never set and output mirrors will never be used.

Otherwise, I think it looks good.  This series is a nice cleanup, so thank you.

--Justin





More information about the dev mailing list