tunneling: Don't match on source IP address for native tunnels.

When doing native tunneling, we look at packets destined to the
local port to see if they match tunnel protocols that we should
intercept. The criteria are IP protocol, destination UDP port, etc.

However, we also look at the source IP address of the packets. This
should be a function of the port-based tunnel layer and not the
tunnel receive code itself. For comparison, the kernel tunnel code
has no idea about the IP addresses of its link partners. If port
based tunnel is desired, it can be handled using the normal port
tunnel layer, regardless of whether the packets originally came
from userspace or the kernel.

For port based tunneling, this bug has no effect - the check is
simply redundant. However, it breaks flow-based native tunnels
because the remote IP address is not known at port creation time.

CC: Pravin Shelar <pshelar@nicira.com>
Reported-by: David Griswold <David.Griswold@overturenetworks.com>
Tested-by: David Griswold <David.Griswold@overturenetworks.com>
Signed-off-by: Jesse Gross <jesse@nicira.com>

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 2602db5..77f54e7 100644 (file)
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -56,7 +56,7 @@ tnl_port_free(struct tnl_port_in *p)
 }
 
 static void
-tnl_port_init_flow(struct flow *flow, ovs_be32 ip_dst, ovs_be16 udp_port)
+tnl_port_init_flow(struct flow *flow, ovs_be16 udp_port)
 {
     memset(flow, 0, sizeof *flow);
     flow->dl_type = htons(ETH_TYPE_IP);
@@ -66,21 +66,17 @@ tnl_port_init_flow(struct flow *flow, ovs_be32 ip_dst, ovs_be16 udp_port)
         flow->nw_proto = IPPROTO_GRE;
     }
     flow->tp_dst = udp_port;
-    /* When matching on incoming flow from remove tnl end point,
-     * our dst ip address is source ip for them. */
-    flow->nw_src = ip_dst;
 }
 
 void
-tnl_port_map_insert(odp_port_t port, ovs_be32 ip_dst, ovs_be16 udp_port,
-                    const char dev_name[])
+tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port, const char dev_name[])
 {
     const struct cls_rule *cr;
     struct tnl_port_in *p;
     struct match match;
 
     memset(&match, 0, sizeof match);
-    tnl_port_init_flow(&match.flow, ip_dst, udp_port);
+    tnl_port_init_flow(&match.flow, udp_port);
 
     ovs_mutex_lock(&mutex);
     do {
@@ -97,7 +93,6 @@ tnl_port_map_insert(odp_port_t port, ovs_be32 ip_dst, ovs_be16 udp_port,
         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_src = OVS_BE32_MAX;
 
         cls_rule_init(&p->cr, &match, 0, CLS_MIN_VERSION); /* Priority == 0. */
         ovs_refcount_init(&p->ref_cnt);
@@ -123,12 +118,12 @@ tnl_port_unref(const struct cls_rule *cr)
 }
 
 void
-tnl_port_map_delete(ovs_be32 ip_dst, ovs_be16 udp_port)
+tnl_port_map_delete(ovs_be16 udp_port)
 {
     const struct cls_rule *cr;
     struct flow flow;
 
-    tnl_port_init_flow(&flow, ip_dst, udp_port);
+    tnl_port_init_flow(&flow, udp_port);
 
     cr = classifier_lookup(&cls, CLS_MAX_VERSION, &flow, NULL);
     tnl_port_unref(cr);

diff --git a/lib/tnl-ports.h b/lib/tnl-ports.h
index 37a689f..8e4911d 100644 (file)
--- a/lib/tnl-ports.h
+++ b/lib/tnl-ports.h
@@ -26,10 +26,10 @@
 
 odp_port_t tnl_port_map_lookup(struct flow *flow, struct flow_wildcards *wc);
 
-void tnl_port_map_insert(odp_port_t port, ovs_be32 ip_dst, ovs_be16 udp_port,
+void tnl_port_map_insert(odp_port_t port, ovs_be16 udp_port,
                          const char dev_name[]);
 
-void tnl_port_map_delete(ovs_be32 ip_dst,  ovs_be16 udp_port);
+void tnl_port_map_delete(ovs_be16 udp_port);
 
 void tnl_port_map_init(void);
 

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index d2ac7c6..42f760e 100644 (file)
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -195,8 +195,7 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
     tnl_port_mod_log(tnl_port, "adding");
 
     if (native_tnl) {
-        tnl_port_map_insert(odp_port, tnl_port->match.ip_dst,
-                            cfg->dst_port, name);
+        tnl_port_map_insert(odp_port, cfg->dst_port, name);
     }
     return true;
 }
@@ -263,7 +262,7 @@ tnl_port_del__(const struct ofport_dpif *ofport) OVS_REQ_WRLOCK(rwlock)
             netdev_get_tunnel_config(tnl_port->netdev);
         struct hmap **map;
 
-        tnl_port_map_delete(tnl_port->match.ip_dst, cfg->dst_port);
+        tnl_port_map_delete(cfg->dst_port);
         tnl_port_mod_log(tnl_port, "removing");
         map = tnl_match_map(&tnl_port->match);
         hmap_remove(*map, &tnl_port->match_node);

diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index b9d3572..d9e8496 100644 (file)
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -11,7 +11,7 @@ AT_CHECK([ovs-vsctl add-port int-br t2 -- set Interface t2 type=vxlan \
                     -- add-port int-br t3 -- set Interface t3 type=vxlan \
                        options:remote_ip=1.1.2.93 options:out_key=flow options:csum=true ofport_request=4\
                     -- add-port int-br t4 -- set Interface t4 type=geneve \
-                       options:remote_ip=1.1.2.92 options:key=123 ofport_request=5\
+                       options:remote_ip=flow options:key=123 ofport_request=5\
                        ], [0])
 
 AT_CHECK([ovs-appctl dpif/show], [0], [dnl
@@ -24,7 +24,7 @@ dummy@ovs-dummy: hit:0 missed:0
                t1 3/3: (gre: key=456, remote_ip=1.1.2.92)
                t2 2/4789: (vxlan: key=123, remote_ip=1.1.2.92)
                t3 4/4789: (vxlan: csum=true, out_key=flow, remote_ip=1.1.2.93)
-               t4 5/6081: (geneve: key=123, remote_ip=1.1.2.92)
+               t4 5/6081: (geneve: key=123, remote_ip=flow)
 ])
 
 AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
@@ -47,10 +47,9 @@ IP               MAC                 Bridge
 
 AT_CHECK([ovs-appctl tnl/ports/show |sort], [0], [dnl
 Listening ports:
-genev_sys_6081 (6081) : eth_type(0x0800),ipv4(src=1.1.2.92,proto=17,frag=no),udp(dst=6081)
-gre_sys (3) : eth_type(0x0800),ipv4(src=1.1.2.92,proto=47,frag=no)
-vxlan_sys_4789 (4789) : eth_type(0x0800),ipv4(src=1.1.2.92,proto=17,frag=no),udp(dst=4789)
-vxlan_sys_4789 (4789) : eth_type(0x0800),ipv4(src=1.1.2.93,proto=17,frag=no),udp(dst=4789)
+genev_sys_6081 (6081) : eth_type(0x0800),ipv4(proto=17,frag=no),udp(dst=6081)
+gre_sys (3) : eth_type(0x0800),ipv4(proto=47,frag=no)
+vxlan_sys_4789 (4789) : eth_type(0x0800),ipv4(proto=17,frag=no),udp(dst=4789)
 ])
 
 dnl Check VXLAN tunnel pop
@@ -93,7 +92,7 @@ AT_CHECK([tail -1 stdout], [0],
 ])
 
 dnl Check Geneve tunnel push
-AT_CHECK([ovs-ofctl add-flow int-br action=5])
+AT_CHECK([ovs-ofctl add-flow int-br "actions=set_field:1.1.2.92->tun_dst,5"])
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=47,tos=0,ttl=64,frag=no)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
   [Datapath actions: tnl_push(tnl_port(6081),header(size=50,type=5,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=17,tos=0,ttl=64,frag=0x40),udp(src=0,dst=6081,csum=0x0),geneve(vni=0x7b)),out_port(100))