Hi Zoltan,

My feeling here is that it is conceptually wrong to do another tunnel lookup if 
the packet is recirculated after it has already been de-tunneled. You are 
trying to fix the symptoms and I am not sure that the more liberal check on 
packet type won't lead to incorrect proper tunnel port lookups.

The actual bridge is stored in the frozen state of the recirculation context, 
and the OF in_port should be stored there as well (if it is not yet). I believe 
a recirculation should never change the bridge and the OF in_port. @Ben: Can 
you confirm?

I think the order of things is wrong in an upcall. First ofproto should check 
if it is a recirculation upcall, and only if it is not, it should derive the 
bridge and the in_port from the flow. Today the first recirculation check 
happens later in upcall_xlate().

Can you look into that option and propose a better way to solve the issue?

Thanks, Jan

> -----Original Message-----
> From: Zoltán Balogh
> Sent: Friday, 08 December, 2017 10:56
> To: [email protected]
> Cc: Zoltán Balogh <[email protected]>; Jan Scheurich 
> <[email protected]>; Ben Pfaff <[email protected]>
> Subject: [PATCH] tunnel: fix tnl_find() after packet_type changed
> 
> During xlate, it can happen that tnl_find() is invoked when flow
> packet_type has been already changed. For instance, pop_mpls and
> resubmit actions should be applied to the packet in overlay bridge after
> packet was received on a legacy_l3 tunnel port.
> In this case, packet is recirculated after pop_mpls, a new tunnel lookup
> is performed in order to find the proper ofproto, however packet_type of
> flow is already PT_ETHERNET while the tunnel port mode is
> NETDEV_PT_LEGACY_L3. So, no tunnel port is found and the packet is
> dropped.
> 
> This fix does an additional tnl_find() if no port is found. It looks for
> L3 tunnel port in case of L2 packet and vice versa.
> 
> Signed-off-by: Zoltan Balogh <[email protected]>
> CC: Jan Scheurich <[email protected]>
> CC: Ben Pfaff <[email protected]>
> Fixes: 875ab13020b1 ("userspace: Handling of versatile tunnel ports")
> ---
>  ofproto/tunnel.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 1676f4d46..d497f026b 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -585,6 +585,19 @@ tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
>                      if (tnl_port) {
>                          return tnl_port;
>                      }
> +
> +                    /* Maybe flow->packet_type has been already changed 
> during
> +                     * xlate, so let's try to find L2 port for L3 packet or
> +                     * L3 port for L2 packet. */
> +                    if (pt_ns(flow->packet_type) == OFPHTN_ETHERTYPE) {
> +                        match.pt_mode = NETDEV_PT_LEGACY_L2;
> +                    } else {
> +                        match.pt_mode = NETDEV_PT_LEGACY_L3;
> +                    }
> +                    tnl_port = tnl_find_exact(&match, map);
> +                    if (tnl_port) {
> +                        return tnl_port;
> +                    }
>                  }
> 
>                  i++;
> --
> 2.14.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to