Maybe ofproto-dpif-upcall.c could be refactored to not need the recirc_id_node
pointer. It is only used for getting the recirc ID and managing recirc_id_node
reference counting. If the reference counting could be moved upstream, the
pointer could be removed. But the idea was to take a reference only if needed
(e.g., when installing a recirculated flow), as taking a reference is extra
cost if we are only executing the packet, or deciding not to install a flow for
any other reason. However, the typical case is that we install a flow for a
miss upcall, so maybe it is not worth optimizing for the exceptional cases?
Acked-by: Jarno Rajahalme <ja...@ovn.org>
> On Jan 18, 2016, at 11:27 PM, Ben Pfaff <b...@ovn.org> wrote:
>
> This will make it possible, in an upcoming commit, to construct a
> recirc_state locally on the stack to pass to xlate_actions(). It would
> also be possible to construct and pass a recirc_id_node on the stack, but
> the translation process only uses the recirc_state anyway. The alternative
> here of having upcall_xlate() know that it can recover the recirc_id_node
> from the recirc_state isn't great either; it's debatable which is the
> better approach.
>
> Signed-off-by: Ben Pfaff <b...@ovn.org>
> ---
> ofproto/ofproto-dpif-rid.h | 6 ++++++
> ofproto/ofproto-dpif-upcall.c | 4 ++--
> ofproto/ofproto-dpif-xlate.c | 11 ++++++++---
> ofproto/ofproto-dpif-xlate.h | 2 +-
> 4 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
> index 04ec037..85ec24a 100644
> --- a/ofproto/ofproto-dpif-rid.h
> +++ b/ofproto/ofproto-dpif-rid.h
> @@ -184,6 +184,12 @@ void recirc_free_ofproto(struct ofproto_dpif *, const
> char *ofproto_name);
>
> const struct recirc_id_node *recirc_id_node_find(uint32_t recirc_id);
>
> +static inline struct recirc_id_node *
> +recirc_id_node_from_state(const struct recirc_state *state)
> +{
> + return CONTAINER_OF(state, struct recirc_id_node, state);
> +}
> +
> static inline bool recirc_id_node_try_ref_rcu(const struct recirc_id_node *n_)
> {
> struct recirc_id_node *node = CONST_CAST(struct recirc_id_node *, n_);
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index b505206..240bd92 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1074,8 +1074,8 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall,
> * upcalls using recirculation ID for which no context can be
> * found). We may still execute the flow's actions even if we
> * don't install the flow. */
> - upcall->recirc = xin.recirc;
> - upcall->have_recirc_ref = recirc_id_node_try_ref_rcu(xin.recirc);
> + upcall->recirc = recirc_id_node_from_state(xin.recirc);
> + upcall->have_recirc_ref =
> recirc_id_node_try_ref_rcu(upcall->recirc);
> }
> } else {
> /* For non-miss upcalls, we are either executing actions (one of which
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 8ab4b2a..184eb46 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4828,9 +4828,14 @@ xlate_in_init(struct xlate_in *xin, struct
> ofproto_dpif *ofproto,
> xin->odp_actions = odp_actions;
>
> /* Do recirc lookup. */
> - xin->recirc = flow->recirc_id
> - ? recirc_id_node_find(flow->recirc_id)
> - : NULL;
> + xin->recirc = NULL;
> + if (flow->recirc_id) {
> + const struct recirc_id_node *node
> + = recirc_id_node_find(flow->recirc_id);
> + if (node) {
> + xin->recirc = &node->state;
> + }
> + }
> }
>
> void
> diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
> index a135d8f..3b06285 100644
> --- a/ofproto/ofproto-dpif-xlate.h
> +++ b/ofproto/ofproto-dpif-xlate.h
> @@ -142,7 +142,7 @@ struct xlate_in {
>
> /* The recirculation context related to this translation, as returned by
> * xlate_lookup. */
> - const struct recirc_id_node *recirc;
> + const struct recirc_state *recirc;
> };
>
> void xlate_ofproto_set(struct ofproto_dpif *, const char *name, struct dpif *,
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
