[ovs-dev] [PATCH 3/9] tnl-ports: Generate mask with correct prerequisites.

[Jarno Rajahalme]

> On Dec 9, 2015, at 6:27 PM, Daniele Di Proietto <diproiettod at vmware.com> wrote:
> 
> We should match on the transport ports only if the tunnel has a UDP
> header.  It doesn't make sense to match on transport port for GRE
> tunnels.
> 
> Also, to match on fragment bits we should use FLOW_NW_FRAG_MASK instead
> of 0xFF.  FLOW_NW_FRAG_MASK is what we get if we convert to the ODP
> netlink format and back.
> 
> Adding the correct masks in the tunnel router classifier helps in making
> sure that the translation generates a masks that respects prerequisites.

delete the extra “a”.

> 
> If the mask has some fields that do not respect prerequisites, the flow
> will get deleted by revalidation, because translating to ODP format and
> back will generate a more generic mask, which will be perceived as too
> generic (compared with the one generated by the translation).
> 

Good catch!

Maybe add a comment that ‘up_port’ is zero for non-UDP tunneling protocols, just to make the conditional a bit clearer?

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

> Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
> ---
> lib/tnl-ports.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
> index 3006a8b..890a8b2 100644
> --- a/lib/tnl-ports.c
> +++ b/lib/tnl-ports.c
> @@ -126,8 +126,10 @@ map_insert(odp_port_t port, struct eth_addr mac, struct in6_addr *addr,
> 
>         match.wc.masks.dl_type = OVS_BE16_MAX;
>         match.wc.masks.nw_proto = 0xff;
> -        match.wc.masks.nw_frag = 0xff;      /* XXX: No fragments support. */
> -        match.wc.masks.tp_dst = OVS_BE16_MAX;
> +        match.wc.masks.nw_frag = FLOW_NW_FRAG_MASK;      /* XXX: No fragments support. */
> +        if (udp_port) {
> +            match.wc.masks.tp_dst = OVS_BE16_MAX;
> +        }
>         if (IN6_IS_ADDR_V4MAPPED(addr)) {
>             match.wc.masks.nw_dst = OVS_BE32_MAX;
>         } else {
> -- 
> 2.1.4
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev