Skip to content

Commit

Permalink
OVN: Don't let peers be set to "<error>" on port bindings.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
putnopvut authored and blp committed Nov 1, 2017
1 parent 173acc1 commit 7b997d4
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions ovn/northd/ovn-northd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -1985,16 +1986,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,
"nat-addresses");
Expand Down

0 comments on commit 7b997d4

Please sign in to comment.