[ovs-dev] ofproto-dpif-xlate: Change assertion to log message.

Message ID 20171207210158.22177-1-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev] ofproto-dpif-xlate: Change assertion to log message.
Related show

Commit Message

Ben Pfaff Dec. 7, 2017, 9:01 p.m.
Until now, compose_output_action__() has asserted that a packet output to
a patch port is not to be truncated.  This commit changes this to an error
that will be included in trace output, for two reasons.  First, this sounds
like only a minor problem to me which doesn't warrant killing the process.
Second, it will be easier to track down the actual problem (if any) if we
can get a trace instead of a segfault.

Reported-by: Kevin Lin <kevin@kelda.io>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045832.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ofproto/ofproto-dpif-xlate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Gregory Rose Dec. 7, 2017, 9:55 p.m. | #1
On 12/7/2017 1:01 PM, Ben Pfaff wrote:
> Until now, compose_output_action__() has asserted that a packet output to
> a patch port is not to be truncated.  This commit changes this to an error
> that will be included in trace output, for two reasons.  First, this sounds
> like only a minor problem to me which doesn't warrant killing the process.
> Second, it will be easier to track down the actual problem (if any) if we
> can get a trace instead of a segfault.
>
> Reported-by: Kevin Lin <kevin@kelda.io>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045832.html
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> ---
>   ofproto/ofproto-dpif-xlate.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index fcced344ed8a..60eaf18038ca 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -3701,7 +3701,9 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
>       }
>   
>       if (xport->peer) {
> -       ovs_assert(!truncate);
> +       if (truncate) {
> +           xlate_report_error(ctx, "Cannot truncate output to patch port");
> +       }
>          patch_port_output(ctx, xport, xport->peer);
>          return;
>       }

I agree that is a better approach.

Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Ben Pfaff Dec. 8, 2017, 11:22 p.m. | #2
On Thu, Dec 07, 2017 at 01:55:58PM -0800, Gregory Rose wrote:
> On 12/7/2017 1:01 PM, Ben Pfaff wrote:
> >Until now, compose_output_action__() has asserted that a packet output to
> >a patch port is not to be truncated.  This commit changes this to an error
> >that will be included in trace output, for two reasons.  First, this sounds
> >like only a minor problem to me which doesn't warrant killing the process.
> >Second, it will be easier to track down the actual problem (if any) if we
> >can get a trace instead of a segfault.
> >
> >Reported-by: Kevin Lin <kevin@kelda.io>
> >Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2017-December/045832.html
> >Signed-off-by: Ben Pfaff <blp@ovn.org>
> >---
> >  ofproto/ofproto-dpif-xlate.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> >index fcced344ed8a..60eaf18038ca 100644
> >--- a/ofproto/ofproto-dpif-xlate.c
> >+++ b/ofproto/ofproto-dpif-xlate.c
> >@@ -3701,7 +3701,9 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
> >      }
> >      if (xport->peer) {
> >-       ovs_assert(!truncate);
> >+       if (truncate) {
> >+           xlate_report_error(ctx, "Cannot truncate output to patch port");
> >+       }
> >         patch_port_output(ctx, xport, xport->peer);
> >         return;
> >      }
> 
> I agree that is a better approach.
> 
> Reviewed-by: Greg Rose <gvrose8192@gmail.com>

Thanks, I applied this to master.

Patch

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index fcced344ed8a..60eaf18038ca 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3701,7 +3701,9 @@  compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
     }
 
     if (xport->peer) {
-       ovs_assert(!truncate);
+       if (truncate) {
+           xlate_report_error(ctx, "Cannot truncate output to patch port");
+       }
        patch_port_output(ctx, xport, xport->peer);
        return;
     }