[ovs-dev] [PATCH] ofproto-dpif-xlate: Remove assertion for truncated

Iwase Yusuke iwase.yusuke0 at gmail.com
Tue Oct 10 07:28:38 UTC 2017


Hi Andy,

Thank you for your quick response.

Your patch looks good to me.
I couldn't judge whether we can "truncate" completely or not, that sounds great!

Thanks,
Iwase


On 2017年10月10日 16:21, Andy Zhou wrote:
> On Mon, Oct 9, 2017 at 8:18 PM, Iwase Yusuke <iwase.yusuke0 at gmail.com> wrote:
>> Hi Ben and Andy,
>>
>> Thank you very much!
>> I'm looking forward to review comments.
>>
>>
> Hi, Iwase,
> 
> I miss interpreted max_len in the spec, and thought (wrongly) that
> only the output controller action should have a non-zero value.
> Apparently, this
> is too restrictive as you have pointed out.
> 
> In terms of the fix. I think we can remove the 'truncate" variable
> completely. If the following incremental looks O.K. to you, I'd like
> to fold it in before pushing your patch to master.
> 
> Thanks for reporting the bug!
> 
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index d320d570b304..a492c2215851 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4829,31 +4829,26 @@ xlate_output_action(struct xlate_ctx *ctx,
>                       bool is_last_action)
>   {
>       ofp_port_t prev_nf_output_iface = ctx->nf_output_iface;
> -    bool truncate = max_len != 0;
> 
>       ctx->nf_output_iface = NF_OUT_DROP;
> 
>       switch (port) {
>       case OFPP_IN_PORT:
>           compose_output_action(ctx, ctx->xin->flow.in_port.ofp_port, NULL,
> -                              is_last_action, truncate);
> +                              is_last_action, false);
>           break;
>       case OFPP_TABLE:
> -        ovs_assert(!truncate);
>           xlate_table_action(ctx, ctx->xin->flow.in_port.ofp_port,
>                              0, may_packet_in, true, false, false,
>                              do_xlate_actions);
>           break;
>       case OFPP_NORMAL:
> -        ovs_assert(!truncate);
>           xlate_normal(ctx);
>           break;
>       case OFPP_FLOOD:
> -        ovs_assert(!truncate);
>           flood_packets(ctx, false, is_last_action);
>           break;
>       case OFPP_ALL:
> -        ovs_assert(!truncate);
>           flood_packets(ctx, true, is_last_action);
>           break;
>       case OFPP_CONTROLLER:
> @@ -4869,7 +4864,7 @@ xlate_output_action(struct xlate_ctx *ctx,
>       case OFPP_LOCAL:
>       default:
>           if (port != ctx->xin->flow.in_port.ofp_port) {
> -            compose_output_action(ctx, port, NULL, is_last_action, truncate);
> +            compose_output_action(ctx, port, NULL, is_last_action, false);
>           } else {
>               xlate_report(ctx, OFT_WARN, "skipping output to input port");
>           }
> 


More information about the dev mailing list