[ovs-dev] [PATCH] ofproto-dpif-xlate: Added logging for when sending out an in port

[Ben Pfaff]

On Tue, Aug 07, 2018 at 10:37:35AM -0700, Zak Whittington wrote:
> VMware-BZ: 2158607
> Signed-off-by: Zak Whittington <zwhitt.vmware at gmail.com>
> ---
>  ofproto/ofproto-dpif-xlate.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 01f1faf..9b36536 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -5007,6 +5007,18 @@ xlate_output_action(struct xlate_ctx *ctx, ofp_port_t port,
>          if (port != ctx->xin->flow.in_port.ofp_port) {
>              compose_output_action(ctx, port, NULL, is_last_action, truncate);
>          } else {
> +            static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> +            if (!VLOG_DROP_INFO(&rl)) {
> +                struct ds s = DS_EMPTY_INITIALIZER;
> +                ds_put_cstr(&s,
> +                    "Skipping output to input port while processing: ");
> +                flow_format(&s, &ctx->base_flow, NULL);
> +                ds_put_format(&s, " on bridge %s", ctx->xbridge->name);
> +                VLOG_INFO("%s", ds_cstr(&s));
> +                ds_destroy(&s);
> +            }
> +
> +            /* For when tracing only: */

Thanks for v2!  I have a few remaining comments.

I suggest putting the flow at the very end of the log message: because
it is long, it is hard for the reader to spot anything that come after
it.  (I see that there's a case in this file where we do it the same way
you did.  I'll send a fix for that in a minute.)

I suggest only emitting a message if we have a packet, that is, if
ctx->xin->packet is nonnull.  Otherwise, we'll emit this message even
when revalidating, which is likely to confuse readers.

The two cases look the same, so I'd add a helper.  Maybe we should have
an xlate_report_info() function similar to xlate_report_error() but with
a lower log level.

Thanks,

Ben.