Thanks for merging this!
I do agree that we should continue to try to simplify this further.
Perhaps we can have a look at it in the new attempt to upstream our previously
reverted patch to avoid recirculation at TX to tunnel port, which refactors
some of the patch-port ("output to peer") logic.
BR, Jan
> -----Original Message-----
> From: ovs-dev-boun...@openvswitch.org
> [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff
> Sent: Friday, 19 May, 2017 06:42
> To: Zoltán Balogh <zoltan.bal...@ericsson.com>
> Cc: 'd...@openvswitch.org' <d...@openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH v6 4/5] ofproto-dpif-xlate: refactor
> compose_output_action__
>
> On Fri, May 12, 2017 at 11:07:43AM +0000, Zoltán Balogh wrote:
> > From: Jan Scheurich <jan.scheur...@ericsson.com>
> >
> > The very long function compose_output_action__() has been re-factored to
> > make
> > the different cases for output to patch-port, native tunnel port, kernel
> > tunnel
> > port, recirculation, or termination of a native tunnel at output to LOCAL
> > port
> > clearer. Larger, self-contained blocks have been split out into separate
> > functions.
> >
> > Signed-off-by: Jan Scheurich <jan.scheur...@ericsson.com>
> > Co-authored-by: Zoltan Balogh <zoltan.bal...@ericsson.com>
> >
> > Conflicts:
> > ofproto/ofproto-dpif-xlate.c
>
> I'm happy with this, it's a nice refactoring, although one wonders
> whether it can be taken even farther.
>
> I folded in the following incremental and applied this to master.
>
> --8<--------------------------cut here-------------------------->8--
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 59ef77bf998a..f71a9db0a6b3 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3303,11 +3303,9 @@ check_output_prerequisites(struct xlate_ctx *ctx,
> return true;
> }
>
> -static inline bool
> -terminate_native_tunnel(struct xlate_ctx *ctx,
> - ofp_port_t ofp_port,
> - struct flow *flow,
> - struct flow_wildcards *wc,
> +static bool
> +terminate_native_tunnel(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> + struct flow *flow, struct flow_wildcards *wc,
> odp_port_t *tnl_port)
> {
> *tnl_port = ODPP_NONE;
> @@ -3319,7 +3317,7 @@ terminate_native_tunnel(struct xlate_ctx *ctx,
> *tnl_port = tnl_port_map_lookup(flow, wc);
> }
>
> - return (*tnl_port != ODPP_NONE);
> + return *tnl_port != ODPP_NONE;
> }
>
> static void
> @@ -3535,12 +3533,11 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
> }
>
> if (out_port != ODPP_NONE) {
> -
> /* Commit accumulated flow updates before output. */
> xlate_commit_actions(ctx);
>
> if (xr) {
> - /* Recirculate the packet */
> + /* Recirculate the packet. */
> struct ovs_action_hash *act_hash;
>
> /* Hash action. */
> @@ -3553,7 +3550,6 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
> /* Recirc action. */
> nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_RECIRC,
> xr->recirc_id);
> -
> } else if (is_native_tunnel) {
> /* Output to native tunnel port. */
> build_tunnel_send(ctx, xport, flow, odp_port);
> @@ -3562,8 +3558,7 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
> } else if (terminate_native_tunnel(ctx, ofp_port, flow, wc,
> &odp_tnl_port)) {
> /* Intercept packet to be received on native tunnel port. */
> - nl_msg_put_odp_port(ctx->odp_actions,
> - OVS_ACTION_ATTR_TUNNEL_POP,
> + nl_msg_put_odp_port(ctx->odp_actions, OVS_ACTION_ATTR_TUNNEL_POP,
> odp_tnl_port);
>
> } else {
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
