[ovs-dev] [PATCH] ofp-actions: Properly interpret "output:in_port".

[Jarno Rajahalme]

Acked-by: Jarno Rajahalme <jarno at ovn.org <mailto:jarno at ovn.org>>

Maybe it would be worthwhile elaborating on the subtle difference in the commit message? I.e., “in_port” should match to special OpenFlow port number that outputs to the incoming port, while normally output to the incoming port would be suppressed?

  Jarno

> On Jun 12, 2017, at 8:41 AM, Ben Pfaff <blp at ovn.org> wrote:
> 
> It was being misinterpreted as output:NXM_OF_IN_PORT[], which is subtly
> different and doesn't do anything useful.
> 
> CC: Jarno Rajahalme <jarno at ovn.org>
> Fixes: 21b2fa617126 ("ofp-parse: Allow match field names in actions and brackets in matches.")
> Reported-by: nickcooper-zhangtonghao <nic at opencloud.tech>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
> lib/ofp-actions.c  | 36 +++++++++++++++++++-----------------
> tests/ovs-ofctl.at |  2 ++
> 2 files changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index d5e4623d0291..f9140f4e9a7e 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -635,27 +635,29 @@ parse_OUTPUT(const char *arg,
> 
>         output_trunc = ofpact_put_OUTPUT_TRUNC(ofpacts);
>         return parse_truncate_subfield(output_trunc, arg, port_map);
> -    } else {
> -        struct mf_subfield src;
> -        char *error = mf_parse_subfield(&src, arg);
> -        if (!error) {
> -            struct ofpact_output_reg *output_reg;
> +    }
> 
> -            output_reg = ofpact_put_OUTPUT_REG(ofpacts);
> -            output_reg->max_len = UINT16_MAX;
> -            output_reg->src = src;
> -        } else {
> -            free(error);
> -            struct ofpact_output *output;
> +    ofp_port_t port;
> +    if (ofputil_port_from_string(arg, port_map, &port)) {
> +        struct ofpact_output *output = ofpact_put_OUTPUT(ofpacts);
> +        output->port = port;
> +        output->max_len = output->port == OFPP_CONTROLLER ? UINT16_MAX : 0;
> +        return NULL;
> +    }
> 
> -            output = ofpact_put_OUTPUT(ofpacts);
> -            if (!ofputil_port_from_string(arg, port_map, &output->port)) {
> -                return xasprintf("%s: output to unknown port", arg);
> -            }
> -            output->max_len = output->port == OFPP_CONTROLLER ? UINT16_MAX : 0;
> -        }
> +    struct mf_subfield src;
> +    char *error = mf_parse_subfield(&src, arg);
> +    if (!error) {
> +        struct ofpact_output_reg *output_reg;
> +
> +        output_reg = ofpact_put_OUTPUT_REG(ofpacts);
> +        output_reg->max_len = UINT16_MAX;
> +        output_reg->src = src;
>         return NULL;
>     }
> +    free(error);
> +
> +    return xasprintf("%s: output to unknown port", arg);
> }
> 
> static void
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 6afe8f766627..52eaf0320cd5 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -207,6 +207,7 @@ ipv6,actions=ct(commit,nat(src=fe80::20c:29ff:fe88:a18b,random))
> ipv6,actions=ct(commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random))
> ipv6,actions=ct(commit,nat(src=[fe80::20c:29ff:fe88:1]-[fe80::20c:29ff:fe88:a18b]:255-4096,random))
> tcp,actions=ct(commit,nat(src=10.1.1.240-10.1.1.255),alg=ftp)
> +actions=in_port,output:in_port
> ]])
> 
> AT_CHECK([ovs-ofctl parse-flows flows.txt
> @@ -240,6 +241,7 @@ OFPT_FLOW_MOD: ADD ipv6 actions=ct(commit,nat(src=fe80::20c:29ff:fe88:a18b,rando
> OFPT_FLOW_MOD: ADD ipv6 actions=ct(commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random))
> OFPT_FLOW_MOD: ADD ipv6 actions=ct(commit,nat(src=[fe80::20c:29ff:fe88:1]-[fe80::20c:29ff:fe88:a18b]:255-4096,random))
> OFPT_FLOW_MOD: ADD tcp actions=ct(commit,nat(src=10.1.1.240-10.1.1.255),alg=ftp)
> +OFPT_FLOW_MOD: ADD actions=IN_PORT,IN_PORT
> ]])
> AT_CLEANUP
> 
> -- 
> 2.10.2
>