Re: [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 
> ---
>  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()) {
> +struct ds s = DS_EMPTY_INITIALIZER;
> +ds_put_cstr(,
> +"Skipping output to input port while processing: ");
> +flow_format(, >base_flow, NULL);
> +ds_put_format(, " on bridge %s", ctx->xbridge->name);
> +VLOG_INFO("%s", ds_cstr());
> +ds_destroy();
> +}
> +
> +/* 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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

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

[Zak Whittington]

VMware-BZ: 2158607
Signed-off-by: Zak Whittington 
---
 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()) {
+struct ds s = DS_EMPTY_INITIALIZER;
+ds_put_cstr(,
+"Skipping output to input port while processing: ");
+flow_format(, >base_flow, NULL);
+ds_put_format(, " on bridge %s", ctx->xbridge->name);
+VLOG_INFO("%s", ds_cstr());
+ds_destroy();
+}
+
+/* For when tracing only: */
 xlate_report(ctx, OFT_WARN, "skipping output to input port");
 }
 break;
@@ -5092,6 +5104,18 @@ xlate_output_trunc_action(struct xlate_ctx *ctx,
 ctx->xout->slow |= SLOW_ACTION;
 }
 } else {
+static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
+if (!VLOG_DROP_INFO()) {
+struct ds s = DS_EMPTY_INITIALIZER;
+ds_put_cstr(,
+"Skipping output to input port while processing:");
+flow_format(, >base_flow, NULL);
+ds_put_format(, " on bridge %s", ctx->xbridge->name);
+VLOG_INFO("%s", ds_cstr());
+ds_destroy();
+}
+
+/* For when tracing only: */
 xlate_report(ctx, OFT_WARN, "skipping output to input port");
 }
 break;
-- 
2.7.4

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev