Hi Ihar,
See comments in-line
On 3/29/21 9:29 PM, Ihar Hrachyshka wrote:
In some environments, hardware serving the fabric network doesn't
support double 802.1q (0x8100) VLAN tags, but does support 802.11ad
(0x8a88) EthType for two layer VLAN traffic. Specifically, Cisco
hardware UCS VIC was identified affected by this limitation.
With vlan-passthru=true set for a logical switch, VLAN tagged traffic
may be generated by VIFs. This patch allows to support the feature in
affected hardware environments.
Signed-off-by: Ihar Hrachyshka <[email protected]>
---
NEWS | 1 +
controller/physical.c | 33 +++++++++++++----
ovn-nb.xml | 6 +++
tests/ovn.at | 85 +++++++++++++++++++++++++++++++++++++++++++
4 files changed, 117 insertions(+), 8 deletions(-)
diff --git a/NEWS b/NEWS
index 530c5d42f..9e45837c7 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 custom (802.11ad, 0x8a88) EthType for localnet ports.
OVN v21.03.0 - 12 Mar 2021
-------------------------
diff --git a/controller/physical.c b/controller/physical.c
index fa5d0d692..66395bc09 100644
--- a/controller/physical.c
+++ b/controller/physical.c
@@ -609,6 +609,28 @@ put_replace_chassis_mac_flows(const struct simap *ct_zones,
}
}
+static void
+ofpact_put_push_vlan(struct ofpbuf *ofpacts, const struct smap *options, int
tag)
+{
+ const char *ethtype_opt = NULL;
+ if (options) {
+ ethtype_opt = smap_get(options, "ethtype");
+ }
+
+ int ethtype;
+ if (!ethtype_opt || !str_to_int(ethtype_opt, 16, ðtype)) {
+ ethtype = 0x8100;
+ }
+ struct ofpact_push_vlan *push_vlan;
+ push_vlan = ofpact_put_PUSH_VLAN(ofpacts);
+ push_vlan->ethertype = htons(ethtype);
+
+ struct ofpact_vlan_vid *vlan_vid;
+ vlan_vid = ofpact_put_SET_VLAN_VID(ofpacts);
+ vlan_vid->vlan_vid = tag;
+ vlan_vid->push_vlan_if_needed = false;
+}
+
static void
put_replace_router_port_mac_flows(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
@@ -696,10 +718,7 @@ put_replace_router_port_mac_flows(struct ovsdb_idl_index
replace_mac->mac = chassis_mac;
if (tag) {
- struct ofpact_vlan_vid *vlan_vid;
- vlan_vid = ofpact_put_SET_VLAN_VID(ofpacts_p);
- vlan_vid->vlan_vid = tag;
- vlan_vid->push_vlan_if_needed = true;
+ ofpact_put_push_vlan(ofpacts_p, &localnet_port->options, tag);
}
ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
@@ -1195,10 +1214,8 @@ consider_port_binding(struct ovsdb_idl_index
*sbrec_port_binding_by_name,
if (tag) {
/* For containers sitting behind a local vif, tag the packets
* before delivering them. */
- struct ofpact_vlan_vid *vlan_vid;
- vlan_vid = ofpact_put_SET_VLAN_VID(ofpacts_p);
- vlan_vid->vlan_vid = tag;
- vlan_vid->push_vlan_if_needed = true;
+ ofpact_put_push_vlan(
+ ofpacts_p, localnet_port? &localnet_port->options: NULL, tag);
Nit: The '?' and ':' should have a single space before them.
}
ofpact_put_OUTPUT(ofpacts_p)->port = ofport;
if (tag) {
diff --git a/ovn-nb.xml b/ovn-nb.xml
index b0a4adffe..fca22988b 100644
--- a/ovn-nb.xml
+++ b/ovn-nb.xml
@@ -830,6 +830,12 @@
uses its local configuration to determine exactly how to connect to
this locally accessible network, if at all.
</column>
+
+ <column name="options" key="ethtype">
+ Optional. VLAN EtherType field value for encapsulating VLAN
+ headers. Supported values: 8100 (default), 8a88 (QinQ).
+ </column>
+
</group>
<group title="Options for l2gateway ports">
diff --git a/tests/ovn.at b/tests/ovn.at
index 391a8bcd9..03b10b793 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -3164,6 +3164,91 @@ OVN_CLEANUP([hv-1],[hv-2])
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([ovn -- VLAN transparency, passthru=true, multiple hosts, custom
ethtype])
+ovn_start
+
+ethtype=88a8
+
+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
ethtype=0x$ethtype
+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
+
+# create taps on fabric to check vlan encapsulation there
+for i in 1 2; do
+ as hv-$i
+ ovs-vsctl add-port br-phys tap$i -- set Interface tap$i \
+ options:tx_pcap=tap$i-tx.pcap \
+ options:rxq_pcap=tap$i-rx.pcap
+done
+
+for i in 1 2; do
+ : > $i.expected
+done
+: > tap.expected
+
+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.
+ payload=fefefefe
+ local packet=$(echo $dst$src | sed 's/://g')${eth}${payload}
+ vif=vif$inport
+ as hv-$1
+ ovs-appctl netdev-dummy/receive $vif $packet
+ echo $packet >> ${eout#lsp}.expected
$eout is set to ln-100 in both calls to test_packet(), so you are
appending $packet to ln-100.expected. I suspect you meant to use "lsp1"
and "lsp2" here. This way, 1.expected and 2.expected would get populated
with the packet.
+
+ phys_packet=$(echo $dst$src | sed 's/://g')${ethtype}0064${eth}${payload}
+ echo $phys_packet >> tap.expected
+}
+
+test_packet 1 f0:00:00:00:00:03 f0:00:00:00:00:01 8100 ln-100 ln-100
+test_packet 2 f0:00:00:00:00:03 f0:00:00:00:00:02 8100 ln-100 ln-100
+for i in 1 2; do
+ OVN_CHECK_PACKETS_REMOVE_BROADCAST([vif$i-tx.pcap], [$i.expected])
Because of the error in test_packet(), 1.expected and 2.expected are
both empty at this point. It happens that vif1-tx.pcap and vif2-tx.pcap
are also empty. So this check succeeds because it's comparing empty files.
As for why vif1-tx.pcap and vif2-tx.pcap are empty, I don't know. When I
tested locally, I found that vif1-rx.pcap and vif2-rx.pcap had the
expected packets in them, interestingly. Doesn't that seem backwards?
+done
+for i in 1 2; do
+ OVN_CHECK_PACKETS_REMOVE_BROADCAST([tap$i-tx.pcap], [tap.expected])
+done
+
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn -- VLAN transparency, passthru=true])
ovn_start
