[ovs-dev,v2,4/4] ovn: Support multiple router ports per logical switch.
diff mbox

Message ID 1445116064-20782-5-git-send-email-blp@nicira.com
State Superseded
Headers show

Commit Message

Ben Pfaff Oct. 17, 2015, 9:07 p.m. UTC
This allows multiple subnets to be routed directly to a logical switch.

Signed-off-by: Ben Pfaff <blp@nicira.com>
---
 ovn/TODO                |  10 ---
 ovn/northd/ovn-northd.c |  69 ++++++++++-------
 ovn/ovn-nb.xml          |   7 +-
 tests/ovn.at            | 193 +++++++++++++++++++++++++++++-------------------
 4 files changed, 164 insertions(+), 115 deletions(-)

Comments

Justin Pettit Oct. 18, 2015, 6:21 p.m. UTC | #1
> On Oct 17, 2015, at 2:07 PM, Ben Pfaff <blp@nicira.com> wrote:
> 
> This allows multiple subnets to be routed directly to a logical switch.
> 
> Signed-off-by: Ben Pfaff <blp@nicira.com>

Awesome stuff.

Acked-by: Justin Pettit <jpettit@nicira.com>

--Justin

Patch
diff mbox

diff --git a/ovn/TODO b/ovn/TODO
index 7f69508..1f2a73f 100644
--- a/ovn/TODO
+++ b/ovn/TODO
@@ -2,16 +2,6 @@ 
 
 * L3 support
 
-** OVN_Northbound schema
-
-*** Needs to support extra routes
-
-Currently a router port has a single route associated with it, but
-presumably we should support multiple routes.  For connections from
-one router to another, this doesn't seem to matter (just put more than
-one connection between them), but for connections between a router and
-a switch it might matter because a switch has only one router port.
-
 ** New OVN logical actions
 
 *** arp
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index e6e9f3e..a1ad34c 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -237,7 +237,8 @@  struct ovn_datapath {
     ovs_be32 gateway;
 
     /* Logical switch data. */
-    struct ovn_port *router_port;
+    struct ovn_port **router_ports;
+    size_t n_router_ports;
 
     struct hmap port_tnlids;
     uint32_t port_key_hint;
@@ -271,6 +272,7 @@  ovn_datapath_destroy(struct hmap *datapaths, struct ovn_datapath *od)
          * use it. */
         hmap_remove(datapaths, &od->key_node);
         destroy_tnlids(&od->port_tnlids);
+        free(od->router_ports);
         free(od);
     }
 }
@@ -634,7 +636,10 @@  join_logical_ports(struct northd_context *ctx,
 
             peer->peer = op;
             op->peer = peer;
-            op->od->router_port = op;
+            op->od->router_ports = xrealloc(
+                op->od->router_ports,
+                sizeof *op->od->router_ports * (op->od->n_router_ports + 1));
+            op->od->router_ports[op->od->n_router_ports++] = op;
         } else if (op->nbr && op->nbr->peer) {
             char peer_name[UUID_LEN + 1];
             snprintf(peer_name, sizeof peer_name, UUID_FMT,
@@ -1431,18 +1436,7 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
     HMAP_FOR_EACH (op, key_node, ports) {
         if (op->nbr) {
             /* XXX ARP for neighboring router */
-        } else if (op->od->router_port) {
-            const char *peer_name = smap_get(
-                &op->od->router_port->nbs->options, "router-port");
-            if (!peer_name) {
-                continue;
-            }
-
-            struct ovn_port *peer = ovn_port_find(ports, peer_name);
-            if (!peer || !peer->nbr) {
-                continue;
-            }
-
+        } else if (op->od->n_router_ports) {
             for (size_t i = 0; i < op->nbs->n_addresses; i++) {
                 struct eth_addr ea;
                 ovs_be32 ip;
@@ -1450,18 +1444,41 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                 if (ovs_scan(op->nbs->addresses[i],
                              ETH_ADDR_SCAN_FMT" "IP_SCAN_FMT,
                              ETH_ADDR_SCAN_ARGS(ea), IP_SCAN_ARGS(&ip))) {
-                    char *match = xasprintf("reg0 == "IP_FMT, IP_ARGS(ip));
-                    char *actions = xasprintf("eth.src = "ETH_ADDR_FMT"; "
-                                              "eth.dst = "ETH_ADDR_FMT"; "
-                                              "outport = %s; "
-                                              "output;",
-                                              ETH_ADDR_ARGS(peer->mac),
-                                              ETH_ADDR_ARGS(ea),
-                                              peer->json_key);
-                    ovn_lflow_add(lflows, peer->od,
-                                  S_ROUTER_IN_ARP, 200, match, actions);
-                    free(actions);
-                    free(match);
+                    for (size_t j = 0; j < op->od->n_router_ports; j++) {
+                        /* Get the Logical_Router_Port that the Logical_Port is
+                         * connected to, as 'peer'. */
+                        const char *peer_name = smap_get(
+                            &op->od->router_ports[j]->nbs->options,
+                            "router-port");
+                        if (!peer_name) {
+                            continue;
+                        }
+
+                        struct ovn_port *peer
+                            = ovn_port_find(ports, peer_name);
+                        if (!peer || !peer->nbr) {
+                            continue;
+                        }
+
+                        /* Make sure that 'ip' is in 'peer''s network. */
+                        if ((ip ^ peer->network) & peer->mask) {
+                            continue;
+                        }
+
+                        char *match = xasprintf("reg0 == "IP_FMT, IP_ARGS(ip));
+                        char *actions = xasprintf("eth.src = "ETH_ADDR_FMT"; "
+                                                  "eth.dst = "ETH_ADDR_FMT"; "
+                                                  "outport = %s; "
+                                                  "output;",
+                                                  ETH_ADDR_ARGS(peer->mac),
+                                                  ETH_ADDR_ARGS(ea),
+                                                  peer->json_key);
+                        ovn_lflow_add(lflows, peer->od,
+                                      S_ROUTER_IN_ARP, 200, match, actions);
+                        free(actions);
+                        free(match);
+                        break;
+                    }
                 }
             }
         }
diff --git a/ovn/ovn-nb.xml b/ovn/ovn-nb.xml
index ae0d8e2..0bfb587 100644
--- a/ovn/ovn-nb.xml
+++ b/ovn/ovn-nb.xml
@@ -142,9 +142,10 @@ 
         </p>
 
         <p>
-          A given logical switch may have at most one logical port of type
-          <code>router</code>.  (This is not a significant restriction because
-          logical routers may be connected into arbitrary topologies.)
+          If a given logical switch has multiple <code>router</code> ports, the
+          <ref table="Logical_Router_Port"/> rows that they reference must be
+          all on the same <ref table="Logical_Router"/> (for different
+          subnets).
         </p>
 
         <column name="options" key="router-port" type='{"type": "uuid"}'>
diff --git a/tests/ovn.at b/tests/ovn.at
index f72ca7a..6c1dbaf 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -844,51 +844,71 @@  ovn_start
 # Logical network:
 #
 # Three logical switches ls1, ls2, ls3.
-# Three VIFs on each: lp1[123], lp2[123], lp3[123].
-# One logical router lr connected to ls[123].
-ovn-nbctl \
-    -- create Logical_Router name=lr0 ports=@lrp1,@lrp2,@lrp3 \
-    -- --id=@lrp1 create Logical_Router_Port name=lrp1 \
-       network=192.168.1.254/24 mac='"00:00:00:00:ff:01"' \
-    -- --id=@lrp2 create Logical_Router_Port name=lrp2 \
-       network=192.168.2.254/24 mac='"00:00:00:00:ff:02"' \
-    -- --id=@lrp3 create Logical_Router_Port name=lrp3 \
-       network=192.168.3.254/24 mac='"00:00:00:00:ff:03"'
+# One logical router lr0 connected to ls[123],
+# with nine subnets, three per logical switch:
+#
+#    lrp11 on ls1 for subnet 192.168.11.0/24
+#    lrp12 on ls1 for subnet 192.168.12.0/24
+#    lrp13 on ls1 for subnet 192.168.13.0/24
+#    ...
+#    lrp33 on ls3 for subnet 192.168.33.0/24
+#
+# 27 VIFs, 9 per LS, 3 per subnet: lp[123][123][123], where the first two
+# digits are the subnet and the last digit distinguishes the VIF.
 for i in 1 2 3; do
-    lrp_uuid=`ovn-nbctl get Logical_Router_Port lrp$i _uuid`
-    ovn-nbctl \
-        -- lswitch-add ls$i \
-        -- lport-add ls$i lrp$i-attachment \
-        -- set Logical_Port lrp$i-attachment type=router \
-                                             options:router-port=$lrp_uuid \
-                                             addresses='"00:00:00:00:ff:0'$i'"'
+    ovn-nbctl lswitch-add ls$i
     for j in 1 2 3; do
-        ovn-nbctl \
-            -- lport-add ls$i lp$i$j \
-            -- lport-set-addresses lp$i$j "f0:00:00:00:00:$i$j 192.168.$i.$j"
+        for k in 1 2 3; do
+	    ovn-nbctl \
+		-- lport-add ls$i lp$i$j$k \
+		-- lport-set-addresses lp$i$j$k "f0:00:00:00:0$i:$j$k 192.168.$i$j.$k"
+        done
+    done
+done
+
+ovn-nbctl create Logical_Router name=lr0
+for i in 1 2 3; do
+    for j in 1 2 3; do
+	lrp_uuid=`ovn-nbctl \
+	    -- --id=@lrp create Logical_Router_Port name=lrp$i$j \
+	       network=192.168.$i$j.254/24 mac='"00:00:00:00:ff:'$i$j'"' \
+	    -- add Logical_Router lr0 ports @lrp \
+	    -- lport-add ls$i lrp$i$j-attachment`
+	ovn-nbctl \
+	    set Logical_Port lrp$i$j-attachment type=router \
+                             options:router-port=$lrp_uuid \
+                             addresses='"00:00:00:00:ff:'$i$j'"'
     done
 done
 
 # Physical network:
 #
 # Three hypervisors hv[123].
-# lp1[123] spread across hv[123]: lp11 on hv1, lp12 on hv2, lp13 on hv3.
-# lp2[123] spread across hv[23]: lp21 and lp22 on hv2, lp23 on hv3.
-# lp3[123] all on hv3.
+# lp?1[123] spread across hv[123]: lp?11 on hv1, lp?12 on hv2, lp?13 on hv3.
+# lp?2[123] spread across hv[23]: lp?21 and lp?22 on hv2, lp?23 on hv3.
+# lp?3[123] all on hv3.
+
 
 # Given the name of a logical port, prints the name of the hypervisor
 # on which it is located.
 vif_to_hv() {
     case $1 in dnl (
-        11) echo 1 ;; dnl (
-        12 | 21 | 22) echo 2 ;; dnl (
-        13 | 23 | 3?) echo 3 ;;
+        ?11) echo 1 ;; dnl (
+        ?12 | ?21 | ?22) echo 2 ;; dnl (
+        ?13 | ?23 | ?3?) echo 3 ;;
     esac
 }
 
-# Prints the first character of its argument, e.g. "vif_to_ls 12" yields 1.
+# Given the name of a logical port, prints the name of its logical router
+# port, e.g. "vif_to_lrp 123" yields 12.
+vif_to_lrp() {
+    echo ${1%?}
+}
+
+# Given the name of a logical port, prints the name of its logical
+# switch, e.g. "vif_to_ls 123" yields 1.
 vif_to_ls() {
-    echo $1 | sed 's/^\(.\).*/\1/'
+    echo ${1%??}
 }
 
 net_add n1
@@ -900,13 +920,16 @@  for i in 1 2 3; do
 done
 for i in 1 2 3; do
     for j in 1 2 3; do
-        hv=`vif_to_hv $i$j`
-        as hv$hv ovs-vsctl \
-            -- add-port br-int vif$i$j \
-            -- set Interface vif$i$j external-ids:iface-id=lp$i$j \
-                                     options:tx_pcap=hv$hv/vif$i$j-tx.pcap \
-                                     options:rxq_pcap=hv$hv/vif$i$j-rx.pcap \
-                                     ofport-request=$i$j
+        for k in 1 2 3; do
+	    hv=`vif_to_hv $i$j$k`
+	    as hv$hv ovs-vsctl \
+		-- add-port br-int vif$i$j$k \
+		-- set Interface vif$i$j$k \
+		       external-ids:iface-id=lp$i$j$k \
+		       options:tx_pcap=hv$hv/vif$i$j$k-tx.pcap \
+		       options:rxq_pcap=hv$hv/vif$i$j$k-rx.pcap \
+		       ofport-request=$i$j$k
+        done
     done
 done
 
@@ -931,7 +954,9 @@  trim_zeros() {
 }
 for i in 1 2 3; do
     for j in 1 2 3; do
-        : > $i$j.expected
+        for k in 1 2 3; do
+            : > $i$j$k.expected
+	done
     done
 done
 test_ip() {
@@ -942,16 +967,18 @@  test_ip() {
     hv=hv`vif_to_hv $inport`
     as $hv ovs-appctl netdev-dummy/receive vif$inport $packet
     #as $hv ovs-appctl ofproto/trace br-int in_port=$inport $packet
+    in_ls=`vif_to_ls $inport`
+    in_lrp=`vif_to_lrp $inport`
     for outport; do
-        ins=`vif_to_ls $inport`
-        outs=`vif_to_ls $outport`
-        if test $ins = $outs; then
+	out_ls=`vif_to_ls $outport`
+        if test $in_ls = $out_ls; then
             # Ports on the same logical switch receive exactly the same packet.
             echo $packet
         else
             # Routing decrements TTL and updates source and dest MAC
             # (and checksum).
-            echo f000000000${outport}00000000ff0${outs}08004500001c00000000"3f1101"00${src_ip}${dst_ip}0035111100080000
+	    out_lrp=`vif_to_lrp $outport`
+            echo f00000000${outport}00000000ff${out_lrp}08004500001c00000000"3f1101"00${src_ip}${dst_ip}0035111100080000
         fi | trim_zeros >> $outport.expected
     done
 }
@@ -966,25 +993,32 @@  as hv1 ovs-ofctl dump-flows br-int
 #    that packets destined to their input ports are dropped).
 #
 # 2. Broadcast IP packets are delivered to all lports except the input port.
+ip_to_hex() {
+    printf "%02x%02x%02x%02x" "$@"
+}
 for is in 1 2 3; do
     for js in 1 2 3; do
-        bcast=
-        s=$is$js
-        smac=f000000000$s
-        sip=c0a80${is}0${js}
-        for id in 1 2 3; do
-            for jd in 1 2 3; do
-                d=$id$jd
-                dip=c0a80${id}0${jd}
-                if test $is = $id; then dmac=f000000000$d; else dmac=00000000ff0$is; fi
-                if test $d != $s; then unicast=$d; else unicast=; fi
-
-                test_ip $s $smac $dmac $sip $dip $unicast #1
-
-                if test $id = $is && test $jd != $js; then bcast="$bcast $d"; fi
-            done
+	for ks in 1 2 3; do
+	    bcast=
+	    s=$is$js$ks
+	    smac=f00000000$s
+	    sip=`ip_to_hex 192 168 $is$js $ks`
+	    for id in 1 2 3; do
+		for jd in 1 2 3; do
+                    for kd in 1 2 3; do
+			d=$id$jd$kd
+			dip=`ip_to_hex 192 168 $id$jd $kd`
+			if test $is = $id; then dmac=f00000000$d; else dmac=00000000ff$is$js; fi
+			if test $d != $s; then unicast=$d; else unicast=; fi
+
+			test_ip $s $smac $dmac $sip $dip $unicast #1
+
+			if test $id = $is && test $d != $s; then bcast="$bcast $d"; fi
+                    done
+		done
+	    done
+	    test_ip $s $smac ffffffffffff $sip ffffffff $bcast #2
         done
-        test_ip $s $smac ffffffffffff $sip ffffffff $bcast #2
     done
 done
 
@@ -1008,16 +1042,19 @@  test_arp() {
     # Expect to receive the broadcast ARP on the other logical switch ports.
     # (OVN should probably suppress these.)
     local i=`vif_to_ls $inport`
-    local j
+    local j k
     for j in 1 2 3; do
-        if test $i$j != $inport; then
-            echo $request >> $i$j.expected
-        fi
+        for k in 1 2 3; do
+            if test $i$j$k != $inport; then
+                echo $request >> $i$j$k.expected
+            fi
+        done
     done
 
     # Expect to receive the reply, if any.
     if test X$reply_ha != X; then
-        local reply=${sha}00000000ff0${i}08060001080006040002${reply_ha}${tpa}${sha}${spa}
+        lrp=`vif_to_lrp $inport`
+        local reply=${sha}00000000ff${lrp}08060001080006040002${reply_ha}${tpa}${sha}${spa}
         echo $reply >> $inport.expected
     fi
 }
@@ -1034,17 +1071,19 @@  test_arp() {
 # 6. No reply to query for IP address other than router IP.
 for i in 1 2 3; do
     for j in 1 2 3; do
-        smac=f000000000$i$j     # Source MAC
-        sip=c0a80${i}0${j}      # Source IP
-        rip=c0a80${i}fe         # Router IP
-        rmac=00000000ff0$i      # Router MAC
-        test_arp $i$j $smac $sip        $rip        $rmac #3
-        test_arp $i$j $smac c0a80${i}55 $rip        $rmac #4
-        test_arp $i$j $smac 0a123456    $rip        $rmac #5
-        test_arp $i$j $smac $sip        c0a80${i}aa       #6
+        for k in 1 2 3; do
+	    smac=f00000000$i$j$k               # Source MAC
+	    sip=`ip_to_hex 192 168 $i$j $k`    # Source IP
+	    rip=`ip_to_hex 192 168 $i$j 254`   # Router IP
+	    rmac=00000000ff$i$j                # Router MAC
+	    otherip=`ip_to_hex 192 168 $i$j 55` # Some other IP in subnet
+	    test_arp $i$j$k $smac $sip        $rip        $rmac #3
+	    test_arp $i$j$k $smac $otherip    $rip        $rmac #4
+	    test_arp $i$j$k $smac 0a123456    $rip        $rmac #5
+	    test_arp $i$j$k $smac $sip        $otherip          #6
+        done
     done
 done
-
 # Allow some time for packet forwarding.
 # XXX This can be improved.
 sleep 1
@@ -1052,12 +1091,14 @@  sleep 1
 # Now check the packets actually received against the ones expected.
 for i in 1 2 3; do
     for j in 1 2 3; do
-        file=hv`vif_to_hv $i$j`/vif$i$j-tx.pcap
-        echo $file
-        $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $file | trim_zeros > $i$j.packets
-        cp $i$j.expected expout
-        AT_CHECK([cat $i$j.packets], [0], [expout])
-        echo
+        for k in 1 2 3; do
+	    file=hv`vif_to_hv $i$j$k`/vif$i$j$k-tx.pcap
+	    echo $file
+	    $PYTHON "$top_srcdir/utilities/ovs-pcap.in" $file | trim_zeros > $i$j$k.packets
+	    cp $i$j$k.expected expout
+	    AT_CHECK([cat $i$j$k.packets], [0], [expout])
+	    echo
+        done
     done
 done
 AT_CLEANUP