[ovs-dev] [PATCH v3 ovn] Support vlan-passthru for tag=0 logical switch ports

Ihar Hrachyshka ihrachys at redhat.com
Tue Mar 30 17:10:24 UTC 2021


When vlan-passthru is true for LS, for untagged localnet ports, tagged
VLAN traffic originating from VIFs should be passed through intact since
the VLAN header belongs to VIF user, not the localnet port fabric.

This patch adds multinode test cases to cover scenarios where packets
traverse through fabric layer, for both tagged and untagged (tag=0)
localnet ports.

Signed-off-by: Ihar Hrachyshka <ihrachys at redhat.com>

---

v1: initial version.
v2: added ddlog implementation for db transformation.
v2: explain security implications in documentation.
v2: add NEWS entry.
v3: leave passthru unset for port binding when it's false for LS.
v3: removed superfluous test keyword that cripped into patch from
    local experiments.
---
 NEWS                  |   1 +
 controller/physical.c |  12 +++-
 northd/ovn-northd.c   |   5 ++
 northd/ovn_northd.dl  |  10 +++-
 ovn-nb.xml            |   6 +-
 tests/ovn.at          | 131 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 161 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index 530c5d42f..9f281cdc5 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Post-v21.03.0
     (This may take testing and tuning to be effective.)  This version of OVN
     requires DDLog 0.36.
   - Introduce ovn-controller incremetal processing engine statistics
+  - Support vlan-passthru mode for tag=0 localnet ports.
 
 OVN v21.03.0 - 12 Mar 2021
 -------------------------
diff --git a/controller/physical.c b/controller/physical.c
index fa5d0d692..99b23b58d 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -1142,7 +1142,6 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
          * for frames that lack any 802.1Q header later. */
         if (tag || !strcmp(binding->type, "localnet")
             || !strcmp(binding->type, "l2gateway")) {
-            match_set_dl_vlan(&match, htons(tag), 0);
             if (nested_container) {
                 /* When a packet comes from a container sitting behind a
                  * parent_port, we should let it loopback to other containers
@@ -1151,7 +1150,16 @@ consider_port_binding(struct ovsdb_idl_index *sbrec_port_binding_by_name,
                 put_load(1, MFF_LOG_FLAGS, MLF_NESTED_CONTAINER_BIT, 1,
                          ofpacts_p);
             }
-            ofpact_put_STRIP_VLAN(ofpacts_p);
+
+            /* For vlan-passthru switch ports that are untagged, skip
+             * matching/stripping VLAN header that originates from the VIF
+             * itself. */
+            bool passthru = smap_get_bool(&binding->options,
+                                          "vlan-passthru", false);
+            if (!passthru || tag) {
+                match_set_dl_vlan(&match, htons(tag), 0);
+                ofpact_put_STRIP_VLAN(ofpacts_p);
+            }
         }
 
         /* Remember the size with just strip vlan added so far,
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 57df62b92..d3b3db042 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -3008,6 +3008,11 @@ ovn_port_update_sbrec(struct northd_context *ctx,
                 smap_add_format(&options,
                                 "qdisc_queue_id", "%d", queue_id);
             }
+
+            if (smap_get_bool(&op->od->nbs->other_config, "vlan-passthru", false)) {
+                smap_add(&options, "vlan-passthru", "true");
+            }
+
             sbrec_port_binding_set_options(op->sb, &options);
             smap_destroy(&options);
             if (ovn_is_known_nb_lsp_type(op->nbsp->type)) {
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 0063021e1..054ba7f0e 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -125,7 +125,7 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
                       .__type             = lsp.__type,
                       .gateway_chassis    = set_empty(),
                       .ha_chassis_group   = sp.hac_group_uuid,
-                      .options            = lsp.options,
+                      .options            = options,
                       .datapath           = sw.ls._uuid,
                       .parent_port        = lsp.parent_name,
                       .tag                = tag,
@@ -146,6 +146,14 @@ OutProxy_Port_Binding(._uuid              = lsp._uuid,
             Some{name} -> eids.insert("name", name)
         };
         eids
+    },
+    var options = {
+        var options = lsp.options;
+        match (sw.ls.other_config.get("vlan-passthru")) {
+            Some{"true"} -> options.insert("vlan-passthru", "true"),
+            _ -> ()
+        };
+        options
     }.
 
 
diff --git a/ovn-nb.xml b/ovn-nb.xml
index b0a4adffe..a68482320 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -562,7 +562,11 @@
     <group title="Other options">
       <column name="other_config" key="vlan-passthru"
           type='{"type": "boolean"}'>
-        Determines whether VLAN tagged incoming traffic should be allowed.
+        Determines whether VLAN tagged incoming traffic should be allowed. Note
+        that this may have security implications when enabled for a logical
+        switch with a tag=0 localnet port. If not properly isolated from other
+        localnet ports, fabric traffic that belongs to other tagged networks may
+        be passed through such a port.
       </column>
     </group>
 
diff --git a/tests/ovn.at b/tests/ovn.at
index 391a8bcd9..b4a75240c 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3164,6 +3164,137 @@ OVN_CLEANUP([hv-1],[hv-2])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- VLAN transparency, passthru=true, multiple hosts])
+ovn_start
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true
+
+ln_port_name=ln-100
+ovn-nbctl lsp-add ls $ln_port_name "" 100
+ovn-nbctl lsp-set-addresses $ln_port_name unknown
+ovn-nbctl lsp-set-type $ln_port_name localnet
+ovn-nbctl lsp-set-options $ln_port_name network_name=phys-100
+net_add n-100
+
+# two hypervisors, each connected to the same network
+for i in 1 2; do
+    sim_add hv-$i
+    as hv-$i
+    ovs-vsctl add-br br-phys
+    ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys-100:br-phys
+    ovn_attach n-100 br-phys 192.168.0.$i
+done
+
+for i in 1 2; do
+    check ovn-nbctl lsp-add ls lsp$i
+    check ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
+done
+
+for i in 1 2; do
+    as hv-$i
+    ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \
+                                  options:tx_pcap=vif$i-tx.pcap \
+                                  options:rxq_pcap=vif$i-rx.pcap \
+                                  ofport-request=$i
+    OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lsp$i` = xup])
+done
+
+test_packet() {
+    local inport=$1 dst=$2 src=$3 eth=$4 eout=$5 lout=$6
+
+    # First try tracing the packet.
+    uflow="inport==\"lsp$inport\" && eth.dst==$dst && eth.src==$src && eth.type==0x$eth && vlan.present==1"
+    echo "output(\"$lout\");" > expout
+    AT_CAPTURE_FILE([trace])
+    AT_CHECK([ovn-trace --all ls "$uflow" | tee trace | sed '1,/Minimal trace/d'], [0], [expout])
+
+    # Then actually send a packet, for an end-to-end test.
+    local packet=$(echo $dst$src | sed 's/://g')${eth}fefefefe
+    vif=vif$inport
+    as hv-$1
+    ovs-appctl netdev-dummy/receive $vif $packet
+    echo $packet >> ${eout#lsp}.expected
+}
+
+test_packet 1 f0:00:00:00:00:02 f0:00:00:00:00:01 8100 lsp2 lsp2
+test_packet 2 f0:00:00:00:00:01 f0:00:00:00:00:02 8100 lsp1 lsp1
+for i in 1 2; do
+    OVN_CHECK_PACKETS_REMOVE_BROADCAST([vif$i-tx.pcap], [$i.expected])
+done
+
+AT_CLEANUP
+])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- VLAN transparency, passthru=true, multiple hosts, flat/untagged])
+ovn_start
+
+check ovn-nbctl ls-add ls
+check ovn-nbctl --wait=sb add Logical-Switch ls other_config vlan-passthru=true
+
+ln_port_name=ln
+ovn-nbctl lsp-add ls $ln_port_name
+ovn-nbctl lsp-set-addresses $ln_port_name unknown
+ovn-nbctl lsp-set-type $ln_port_name localnet
+ovn-nbctl lsp-set-options $ln_port_name network_name=phys
+net_add n
+
+# two hypervisors, each connected to the same network
+for i in 1 2; do
+    sim_add hv-$i
+    as hv-$i
+    ovs-vsctl add-br br-phys
+    ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
+    ovn_attach n br-phys 192.168.0.$i
+done
+
+for i in 1 2; do
+    check ovn-nbctl lsp-add ls lsp$i
+    check ovn-nbctl lsp-set-addresses lsp$i f0:00:00:00:00:0$i
+done
+
+for i in 1 2; do
+    as hv-$i
+    ovs-vsctl add-port br-int vif$i -- set Interface vif$i external-ids:iface-id=lsp$i \
+                                  options:tx_pcap=vif$i-tx.pcap \
+                                  options:rxq_pcap=vif$i-rx.pcap \
+                                  ofport-request=$i
+    OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up lsp$i` = xup])
+done
+
+for i in 1 2; do
+    : > $i.expected
+done
+
+test_packet() {
+    local inport=$1 dst=$2 src=$3 eth=$4 eout=$5 lout=$6
+
+    # First try tracing the packet.
+    uflow="inport==\"lsp$inport\" && eth.dst==$dst && eth.src==$src && eth.type==0x$eth && vlan.present==1"
+    echo "output(\"$lout\");" > expout
+    AT_CAPTURE_FILE([trace])
+    AT_CHECK([ovn-trace --all ls "$uflow" | tee trace | sed '1,/Minimal trace/d'], [0], [expout])
+
+    # Then actually send a packet, for an end-to-end test.
+    local packet=$(echo $dst$src | sed 's/://g')${eth}fefefefe
+    vif=vif$inport
+    as hv-$1
+    ovs-appctl netdev-dummy/receive $vif $packet
+    echo $packet >> ${eout#lsp}.expected
+}
+
+test_packet 1 f0:00:00:00:00:02 f0:00:00:00:00:01 8100 lsp2 lsp2
+test_packet 2 f0:00:00:00:00:01 f0:00:00:00:00:02 8100 lsp1 lsp1
+
+for i in 1 2; do
+    OVN_CHECK_PACKETS_REMOVE_BROADCAST([vif$i-tx.pcap], [$i.expected])
+done
+
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([ovn -- VLAN transparency, passthru=true])
 ovn_start
-- 
2.30.2



More information about the dev mailing list