[ovs-dev] [PATCH] OVN: Don't let peers be set to "<error>" on port bindings.

Mark Michelson mmichels at redhat.com
Thu Oct 26 18:09:01 UTC 2017

There are a couple of places in ovn-northd that set the "peer" option on
certain ports to "<error>" in certain cases. In every case where a peer is
looked up on a port binding, the code performs a NULL check in order to
ensure a peer exists. None check for the "<error>" string. They assume that the
presence of a peer string means a peer is defined and all is well.

In the past (OVS 2.6 series), this sometimes led to patch ports being created
in ovs that had names like "patch-ro-to-<error>". This particular problem
resolved itself in OVS 2.7 since such patch ports were no longer automatically
created. However, by naming the peer "<error>" the seeds are still sown for
similar issues to occur.

The solution this patch suggests is to no longer set the "peer" option
on a port binding to "<error>". Instead, if no peer can be set, then we
set no peer. Since other code is already equipped to deal with this,
this poses no problem.

Signed-off-by: Mark Michelson <mmichels at redhat.com>
 ovn/northd/ovn-northd.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 2db238073..ec981541e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -1924,8 +1924,9 @@ ovn_port_update_sbrec(struct northd_context *ctx,
             smap_add(&new, "distributed-port", op->nbrp->name);
         } else {
-            const char *peer = op->peer ? op->peer->key : "<error>";
-            smap_add(&new, "peer", peer);
+            if (op->peer) {
+                smap_add(&new, "peer", op->peer->key);
+            }
             if (chassis_name) {
                 smap_add(&new, "l3gateway-chassis", chassis_name);
@@ -1978,16 +1979,20 @@ ovn_port_update_sbrec(struct northd_context *ctx,
                 sbrec_port_binding_set_type(op->sb, "patch");
-            const char *router_port = smap_get_def(&op->nbsp->options,
-                                                   "router-port", "<error>");
-            struct smap new;
-            smap_init(&new);
-            smap_add(&new, "peer", router_port);
-            if (chassis) {
-                smap_add(&new, "l3gateway-chassis", chassis);
+            const char *router_port = smap_get(&op->nbsp->options,
+                                               "router-port");
+            if (router_port || chassis) {
+                struct smap new;
+                smap_init(&new);
+                if (router_port) {
+                    smap_add(&new, "peer", router_port);
+                }
+                if (chassis) {
+                    smap_add(&new, "l3gateway-chassis", chassis);
+                }
+                sbrec_port_binding_set_options(op->sb, &new);
+                smap_destroy(&new);
-            sbrec_port_binding_set_options(op->sb, &new);
-            smap_destroy(&new);
             const char *nat_addresses = smap_get(&op->nbsp->options,

More information about the dev mailing list