[ovs-dev] [PATCH/RFC 4/8] ofproto-dpif: Add recirc_id field to struct rule_dpif

[Andy Zhou]

Simon, I am not sure If I understand how recirc_id should be managed for MPLS.

Why should rule allocate and free recirc_id by itself, rather that
some other entity managing them?  In the bond case, bond is one
managing recirc_ids. one recirc_id per bond.  I'd imaging some entity
in the user space should keep track of the mpls labels and their
mapping to recirc_id?


On Tue, Mar 25, 2014 at 2:24 PM, Simon Horman <horms at verge.net.au> wrote:
> This is to allow a recirculation id to be associated with a rule
> in the case that its actions cause recirculation.
>
> In such a case if the recirc_id field is non-zero then that value should be
> used, otherwise a value should be obtained using
> ofproto_dpif_alloc_recirc_id and saved in recirc_id field.
>
> When destructing the rule if the recirc_id field is non-zero then
> the associated internal flow should be deleted.
>
> This is in preparation for using the same helper as part of support
> for using recirculation in conjunction series of actions including
> with MPLS actions that are currently not able to be translated.
>
> Signed-off-by: Simon Horman <horms at verge.net.au>
> ---
>  ofproto/ofproto-dpif.c | 27 +++++++++++++++++++++++++++
>  ofproto/ofproto-dpif.h |  1 +
>  2 files changed, 28 insertions(+)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 05f7bca..f239735 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -89,6 +89,12 @@ struct rule_dpif {
>       *   recently been processed by a revalidator. */
>      struct ovs_mutex stats_mutex;
>      struct dpif_flow_stats stats OVS_GUARDED;
> +
> +    /* If non-zero then the recirculation id that has
> +     * been allocated for use with this rule.
> +     * The recirculation id and associated internal flow should
> +     * be freed when the rule is freed */
> +    uint32_t recirc_id;
>  };
>
>  static void rule_get_stats(struct rule *, uint64_t *packets, uint64_t *bytes,
> @@ -3154,6 +3160,19 @@ rule_dpif_get_actions(const struct rule_dpif *rule)
>      return rule_get_actions(&rule->up);
>  }
>
> +/* Returns 'rule''s recirculation id. */
> +uint32_t
> +rule_dpif_get_recirc_id(struct rule_dpif *rule)
> +    OVS_REQUIRES(rule->up.mutex)
> +{
> +    if (!rule->recirc_id) {
> +        struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> +
> +        rule->recirc_id = ofproto_dpif_alloc_recirc_id(ofproto);
> +    }
> +    return rule->recirc_id;
> +}
> +
>  static uint8_t
>  rule_dpif_lookup__ (struct ofproto_dpif *ofproto, const struct flow *flow,
>                      struct flow_wildcards *wc, struct rule_dpif **rule)
> @@ -3376,6 +3395,8 @@ rule_construct(struct rule *rule_)
>      rule->stats.n_packets = 0;
>      rule->stats.n_bytes = 0;
>      rule->stats.used = rule->up.modified;
> +    rule->recirc_id = 0;
> +
>      return 0;
>  }
>
> @@ -3399,7 +3420,13 @@ static void
>  rule_destruct(struct rule *rule_)
>  {
>      struct rule_dpif *rule = rule_dpif_cast(rule_);
> +
>      ovs_mutex_destroy(&rule->stats_mutex);
> +    if (rule->recirc_id) {
> +        struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto);
> +
> +        ofproto_dpif_free_recirc_id(ofproto, rule->recirc_id);
> +    }
>  }
>
>  static void
> diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
> index 6d21bac..9a07d34 100644
> --- a/ofproto/ofproto-dpif.h
> +++ b/ofproto/ofproto-dpif.h
> @@ -101,6 +101,7 @@ bool rule_dpif_is_internal(const struct rule_dpif *);
>  uint8_t rule_dpif_get_table(const struct rule_dpif *);
>
>  struct rule_actions *rule_dpif_get_actions(const struct rule_dpif *);
> +uint32_t rule_dpif_get_recirc_id(struct rule_dpif *rule);
>
>  ovs_be64 rule_dpif_get_flow_cookie(const struct rule_dpif *rule);
>
> --
> 1.8.4
>