In compose_output_action__(), terminate_native_tunnel() is called
to test if a pkt matches a tnl port. If yes, a TUNNEL_POP action
is appended. Instead of outputing the pkt to the origin port, pkt
is redirected into the tnl port.

pkt metadata(i.e. nw_proto, dst_ip, dst_port) is considered when
testing a pkt regardless of the out port until Ilya's patch[1].
Patch[1] adds the condition that terminal ports must have IP
address. This patch is to make addtional port ip check that port
ip address must match the pkt dst ip. This check covers the mirror
port case:
To tcpdump underlay pkt, we may create mirror port of dpdk port.
When compose_output_action__() is called on the mirror port,
pkt is redirected into tnl port again. As a result, ping shows
DUP. fast path flow has two tnl_pop actions.

```
[root@ovs1 vagrant]# ping 172.16.100.2
PING 172.16.100.2 (172.16.100.2) 56(84) bytes of data.
64 bytes from 172.16.100.2: icmp_seq=1 ttl=64 time=0.606 ms
64 bytes from 172.16.100.2: icmp_seq=1 ttl=64 time=0.617 ms (DUP!)
```

```
[root@ovs ovs]# ovs-appctl dpctl/dump-flows |grep tnl_pop
recirc_id(0),in_port(3),packet_type(ns=0,id=0),eth(src=52:54:00:a6:c6:de,dst=ea:b4:09:72:44:4c),eth_type(0x0800),ipv4(dst=192.168.124.33,proto=17,frag=no),udp(dst=8472),
 packets:1, bytes:148, used:0.637s, actions:tnl_pop(2),tnl_pop(2)
```

[1] (d584bb2b6a1 "ofproto-dpif-xlate: Terminate native tunnels only on ports 
with IP addresses.")

Signed-off-by: lic121 <lic...@chinatelecom.cn>
---
 ofproto/ofproto-dpif-xlate.c | 45 +++++++++++++++++++++++++++-------------
 tests/tunnel-push-pop.at     | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 14 deletions(-)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 17f7e28..30e636e 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -4107,18 +4107,26 @@ is_neighbor_reply_correct(const struct xlate_ctx *ctx, 
const struct flow *flow)
 }
 
 static bool
-xport_has_ip(const struct xport *xport)
+xport_has_ip(const struct xport *xport, const struct in6_addr ip6)
 {
     struct in6_addr *ip_addr, *mask;
     int n_in6 = 0;
+    int i;
+    bool hasip = false;
 
     if (netdev_get_addr_list(xport->netdev, &ip_addr, &mask, &n_in6)) {
         n_in6 = 0;
     } else {
+        for (i = 0; i < n_in6; i++) {
+            if (IN6_ARE_ADDR_EQUAL(ip_addr + i, &ip6)) {
+                hasip = true;
+                break;
+            }
+        }
         free(ip_addr);
         free(mask);
     }
-    return n_in6 ? true : false;
+    return hasip;
 }
 
 static bool
@@ -4127,26 +4135,35 @@ terminate_native_tunnel(struct xlate_ctx *ctx, const 
struct xport *xport,
                         odp_port_t *tnl_port)
 {
     *tnl_port = ODPP_NONE;
+    bool ip_pkt = true;
+    struct in6_addr ip_dst;
+
+    if (flow->dl_type == htons(ETH_TYPE_IP)) {
+        ip_dst = in6_addr_mapped_ipv4(flow->nw_dst);
+    } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
+        ip_dst = flow->ipv6_dst;
+    } else {
+        ip_pkt = false;
+    }
 
     /* XXX: Write better Filter for tunnel port. We can use in_port
      * 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)) {
+     * Not performing a lookup if dst ip not match, to avoid unwildcarding
+     * extra flow fields (dl_dst). Also avoid duplicate tnl_pop. */
+    if ((flow->dl_type == htons(ETH_TYPE_ARP) ||
+                flow->nw_proto == IPPROTO_ICMPV6) &&
+                is_neighbor_reply_correct(ctx, flow)) {
+            tnl_neigh_snoop(flow, wc, ctx->xbridge->name,
+                            ctx->xin->allow_side_effects);
+
+    } else if (ovs_native_tunneling_is_on(ctx->xbridge->ofproto)
+        && ip_pkt && xport_has_ip(xport, ip_dst)) {
         *tnl_port = tnl_port_map_lookup(flow, wc);
 
         /* If no tunnel port was found and it's about an ARP or ICMPv6 packet,
          * do tunnel neighbor snooping. */
-        if (*tnl_port == ODPP_NONE &&
-            (flow->dl_type == htons(ETH_TYPE_ARP) ||
-             flow->nw_proto == IPPROTO_ICMPV6) &&
-             is_neighbor_reply_correct(ctx, flow)) {
-            tnl_neigh_snoop(flow, wc, ctx->xbridge->name,
-                            ctx->xin->allow_side_effects);
-        } else if (*tnl_port != ODPP_NONE &&
+        if (*tnl_port != ODPP_NONE &&
                    ctx->xin->allow_side_effects &&
                    dl_type_is_ip_any(flow->dl_type)) {
             struct eth_addr mac = flow->dl_src;
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index c633441..b3a3bf9 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -808,6 +808,55 @@ NXST_FLOW reply:
 OVS_VSWITCHD_STOP
 AT_CLEANUP
 
+AT_SETUP([tunnel_push_pop - multiple output])
+
+OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy 
ofport_request=1 other-config:hwaddr=aa:55:aa:55:00:00])
+AT_CHECK([ovs-vsctl add-port br0 m0 -- set Interface m0 type=dummy 
ofport_request=2])
+AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br datapath_type=dummy], 
[0])
+AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
+                       options:remote_ip=1.1.2.92 options:key=456 
options:seq=true ofport_request=4], [0])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+  br0:
+    br0 65534/100: (dummy-internal)
+    m0 2/2: (dummy)
+    p0 1/1: (dummy)
+  int-br:
+    int-br 65534/3: (dummy-internal)
+    t1 4/4: (gre: key=456, remote_ip=1.1.2.92, seq=true)
+])
+
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
+])
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr m0 1.1.3.88/24], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.3.92/24 m0], [0], [OK
+])
+AT_CHECK([ovs-ofctl add-flow br0 'arp,priority=1,action=normal'])
+
+dnl Use arp reply to achieve tunnel next hop mac binding
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 
'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
+
+dnl Ouput gre pkt to both br0 port and m0 port
+dnl m0 port to simulate mirror port, or flood case
+AT_CHECK([ovs-ofctl add-flow br0 
'in_port(1),ip,ip_proto=47,priority=99,action=br0,m0'])
+
+AT_CHECK([ovs-appctl netdev-dummy/receive p0 
'recirc_id(0),in_port(1),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(src=1.1.2.92,dst=1.1.2.88,proto=47,tos=0,ttl=64,frag=no)'])
+
+dnl port br0: has the configured ip address of tnl port t1, so 'tnl_pop(4)'
+dnl port m0: does not have a configured tnl ip address, so 'output:2'
+AT_CHECK([ovs-appctl dpctl/dump-flows], [0], [dnl
+flow-dump from the main thread:
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=f8:bc:12:44:34:b6,dst=aa:55:aa:55:00:00),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6),
 packets:0, bytes:0, used:never, actions:100,2
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(dst=aa:55:aa:55:00:00),eth_type(0x0800),ipv4(dst=1.1.2.88,proto=47,frag=no),
 packets:0, bytes:0, used:never, actions:tnl_pop(4),2
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+
 AT_SETUP([tunnel_push_pop - flow translation on unrelated bridges])
 
 OVS_VSWITCHD_START(
-- 
1.8.3.1

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to