[ovs-dev] [PATCH v2 ovn 3/3] binding: fix localnet QoS configuration after I-P

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Mon Sep 21 16:25:37 UTC 2020


After the I-P has been introduced in commit 354bdba51a ("ovn-controller:
I-P for SB port binding and OVS interface in runtime_data"), the QoS on
localnet ports is not properly configured if the ovs interface is marked
with "ovn-egress-iface" flag after the related record in the
logical_switch_port table is set. The issue can be triggered with the
following reproducer:

$ovn-nbctl set Logical_Switch_Port ln-public options:qos_burst=1000
$ovs-vsctl set interface eth0 external-ids:ovn-egress-iface=true

Fix the issue triggering a recomputation after qos is configured for ovs
interface

Fixes: 354bdba51abf ("ovn-controller: I-P for SB port binding and OVS
interface in runtime_data")

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
 controller/binding.c     | 26 +++++++++++++++++
 controller/binding.h     |  1 +
 tests/ovn-performance.at | 13 +++++++++
 tests/system-ovn.at      | 60 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 100 insertions(+)

diff --git a/controller/binding.c b/controller/binding.c
index 107bc28a6..26542ae66 100644
--- a/controller/binding.c
+++ b/controller/binding.c
@@ -1353,6 +1353,13 @@ build_local_bindings(struct binding_ctx_in *b_ctx_in,
                 struct local_iface_node *iface_node =
                     xmalloc(sizeof *iface_node);
                 iface_node->iface_id = xstrdup(iface_id);
+                struct local_iface_node *old_iface_node = shash_find_data(
+                        b_ctx_out->local_ifaces,
+                        iface_rec->name);
+                if (old_iface_node) {
+                    iface_node->qos_egress_iface =
+                        old_iface_node->qos_egress_iface;
+                }
                 iface_node = shash_replace(b_ctx_out->local_ifaces,
                                            iface_rec->name, iface_node);
                 if (iface_node) {
@@ -1700,6 +1707,12 @@ consider_iface_claim(const struct ovsrec_interface *iface_rec,
 
     struct local_iface_node *iface_node = xmalloc(sizeof *iface_node);
     iface_node->iface_id = xstrdup(iface_id);
+    struct local_iface_node *old_iface_node = shash_find_data(
+            b_ctx_out->local_ifaces,
+            iface_rec->name);
+    if (old_iface_node) {
+        iface_node->qos_egress_iface = old_iface_node->qos_egress_iface;
+    }
     iface_node = shash_replace(b_ctx_out->local_ifaces,
                                iface_rec->name, iface_node);
     if (iface_node) {
@@ -1928,6 +1941,19 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in,
             continue;
         }
 
+        struct local_iface_node *node = shash_find_data(
+                b_ctx_out->local_ifaces, iface_rec->name);
+        bool old_qos_egress = node ? node->qos_egress_iface : false;
+        bool qos_egress = smap_get_bool(&iface_rec->external_ids,
+                                        "ovn-egress-iface", false);
+        if (qos_egress != old_qos_egress) {
+            if (node) {
+                node->qos_egress_iface = qos_egress;
+            }
+            handled = false;
+            break;
+        }
+
         const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id");
         int64_t ofport = iface_rec->n_ofport ? *iface_rec->ofport : 0;
         if (iface_id && ofport > 0) {
diff --git a/controller/binding.h b/controller/binding.h
index a1e0f1ccf..9e3e1f512 100644
--- a/controller/binding.h
+++ b/controller/binding.h
@@ -56,6 +56,7 @@ struct binding_ctx_in {
 
 struct local_iface_node {
     char *iface_id;
+    bool qos_egress_iface;
 };
 
 struct binding_ctx_out {
diff --git a/tests/ovn-performance.at b/tests/ovn-performance.at
index 3010860d5..6cc5b2174 100644
--- a/tests/ovn-performance.at
+++ b/tests/ovn-performance.at
@@ -247,6 +247,8 @@ for i in 1 2 3; do
     ovs-vsctl set open . external_ids:ovn-bridge-mappings="public:br-ex"
     j=$((i + 2))
     ovn_attach n1 br-phys 192.168.0.$j
+    ip link add vgw$i type dummy
+    ovs-vsctl add-port br-ex vgw$i
 done
 
 # Wait for the tunnel ports to be created and up.
@@ -477,6 +479,17 @@ OVN_CONTROLLER_EXPECT_HIT(
     [ovn-nbctl --wait=hv lrp-set-gateway-chassis lr1-public gw3 10 && ovn-nbctl --wait=hv sync]
 )
 
+# create QoS rule
+OVN_CONTROLLER_EXPECT_NO_HIT(
+    [hv1 hv2 gw1 gw2 gw3], [lflow_run],
+    [ovn-nbctl --wait=hv set Logical_Switch_Port ln-public options:qos_burst=1000]
+)
+
+OVN_CONTROLLER_EXPECT_HIT(
+    [gw1], [lflow_run],
+    [as gw1 ovs-vsctl set interface vgw1 external-ids:ovn-egress-iface=true]
+)
+
 # Make gw2 master. There is remote possibility that full recompute
 # triggers for gw2 after it becomes master. Most of the time
 # there will be no recompute.
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 4f72754c6..f779947f4 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -5397,3 +5397,63 @@ as
 OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
 /.*terminating with signal 15.*/d"])
 AT_CLEANUP
+
+AT_SETUP([ovn -- egress qos])
+AT_KEYWORDS([ovn-egress-qos])
+
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_BR([br-int])
+ADD_BR([br-ext])
+
+ovs-ofctl add-flow br-ext action=normal
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+ovn-nbctl ls-add sw0
+
+ADD_NAMESPACES(sw01)
+ADD_VETH(sw01, sw01, br-int, "192.168.1.2/24", "f0:00:00:01:02:03")
+ovn-nbctl lsp-add sw0 sw01 \
+    -- lsp-set-addresses sw01 "f0:00:00:01:02:03 192.168.1.2"
+
+ADD_NAMESPACES(public)
+ADD_VETH(public, public, br-ext, "192.168.2.2/24", "f0:00:00:01:02:05")
+
+AT_CHECK([ovs-vsctl set Open_vSwitch . external-ids:ovn-bridge-mappings=phynet:br-ext])
+ovn-nbctl lsp-add sw0 public \
+        -- lsp-set-addresses public unknown \
+        -- lsp-set-type public localnet \
+        -- lsp-set-options public network_name=phynet
+
+AT_CHECK([ovn-nbctl set Logical_Switch_Port public options:qos_burst=1000])
+AT_CHECK([ovs-vsctl set interface ovs-public external-ids:ovn-egress-iface=true])
+AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public'])
+
+AT_CHECK([ovn-nbctl remove Logical_Switch_Port public options qos_burst=1000])
+AT_CHECK([tc qdisc show | grep -q 'htb 1: dev ovs-public'],[1])
+
+kill $(pidof ovn-controller)
+
+as ovn-sb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as ovn-nb
+OVS_APP_EXIT_AND_WAIT([ovsdb-server])
+
+as northd
+OVS_APP_EXIT_AND_WAIT([ovn-northd])
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
+/.*terminating with signal 15.*/d"])
+AT_CLEANUP
-- 
2.26.2



More information about the dev mailing list