ofproto-dpif-xlate: Terminate native tunnels only on ports with IP addresses.

Commit dc0bd12f5b04 removed restriction that tunnel endpoint must be a
bridge port.  So, currently OVS has to check if the native tunnel needs
to be terminated regardless of the output port.  Unfortunately, there
is a side effect: tnl_port_map_lookup() always adds at least 'dl_dst'
match to the megaflow that ends up in the corresponding datapath flow.
And since tunneling works on L3 level and not restricted by any
particular bridge, this extra match criteria is added to every
datapath flow on every bridge even if that bridge cannot be part of
a tunnel processing.

For example, if OVS has at least one tunnel configured and we're
adding a completely separate bridge with 2 ports and simple rules
to forward packets between two ports, there still will be a match on
a destination mac address:

 1. <create a tunnel configuration in OVS>
 2. ovs-vsctl add-br br-non-tunnel -- set bridge datapath_type=netdev
 3. ovs-vsctl add-port br-non-tunnel port0
           -- add-port br-non-tunnel port1
 4. ovs-ofctl del-flows br-non-tunnel
 5. ovs-ofctl add-flow br-non-tunnel in_port=port0,actions=port1
 6. ovs-ofctl add-flow br-non-tunnel in_port=port1,actions=port0

 # ovs-appctl ofproto/trace br-non-tunnel in_port=port0

 Flow: in_port=1,vlan_tci=0x0000,
       dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,dl_type=0x0000

 bridge("br-non-tunnel")
 -----------------------
  0. in_port=1, priority 32768
     output:2

 Final flow: unchanged
 Megaflow: recirc_id=0,eth,in_port=1,dl_dst=00:00:00:00:00:00,dl_type=0x0000
 Datapath actions: 5                 ^^^^^^^^^^^^^^^^^^^^^^^^

This increases the number of upcalls and installed datapath flows,
since separate flow needs to be installed per destination MAC, reducing
the switching performance.  This also blocks datapath performance
optimizations that are based on the datapath flow simplicity.

In general, in order to be a tunnel endpoint, port has to have an IP
address.  Hence native tunnel termination should be attempted only
for such ports.  This allows to avoid extra matches in most cases.

Fixes: dc0bd12f5b04 ("userspace: Enable non-bridge port as tunnel endpoint.")
Reported-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2021-October/388904.html
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Mike Pattrick <mkp@redhat.com>

1 files changed, 26 insertions, 5 deletions

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8ee6f5a4c..88cdc09a0 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4081,14 +4081,35 @@ is_neighbor_reply_correct(const struct xlate_ctx *ctx, const struct flow *flow)
 }
 
 static bool
-terminate_native_tunnel(struct xlate_ctx *ctx, struct flow *flow,
-                        struct flow_wildcards *wc, odp_port_t *tnl_port)
+xport_has_ip(const struct xport *xport)
+{
+    struct in6_addr *ip_addr, *mask;
+    int n_in6 = 0;
+
+    if (netdev_get_addr_list(xport->netdev, &ip_addr, &mask, &n_in6)) {
+        n_in6 = 0;
+    } else {
+        free(ip_addr);
+        free(mask);
+    }
+    return n_in6 ? true : false;
+}
+
+static bool
+terminate_native_tunnel(struct xlate_ctx *ctx, const struct xport *xport,
+                        struct flow *flow, struct flow_wildcards *wc,
+                        odp_port_t *tnl_port)
 {
     *tnl_port = ODPP_NONE;
 
     /* XXX: Write better Filter for tunnel port. We can use in_port
-     * in tunnel-port flow to avoid these checks completely. */
-    if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)) {
+     * in tunnel-port flow to avoid these checks completely.
+     *
+     * Port without an IP address cannot be a tunnel termination point.
+     * Not performing a lookup in this case to avoid unwildcarding extra
+     * flow fields (dl_dst). */
+    if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)
+        && xport_has_ip(xport)) {
         *tnl_port = tnl_port_map_lookup(flow, wc);
 
         /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
@@ -4242,7 +4263,7 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
             native_tunnel_output(ctx, xport, flow, odp_port, truncate);
             flow->tunnel = flow_tnl; /* Restore tunnel metadata */
 
-        } else if (terminate_native_tunnel(ctx, flow, wc,
+        } else if (terminate_native_tunnel(ctx, xport, flow, wc,
                                            &odp_tnl_port)) {
             /* Intercept packet to be received on native tunnel port. */
             nl_msg_put_odp_port(ctx->odp_actions, OVS_ACTION_ATTR_TUNNEL_POP,