> On Jul 28, 2016, at 5:01 AM, Ben Pfaff <b...@ovn.org> wrote: > > On Mon, Jul 25, 2016 at 09:19:00AM -0700, nickcooper-zhangtonghao wrote: >> Deletion of a logical router port, which may be an other >> router peer, result in a WARN, because the "ports" variable >> still contains the logical port deleted. This port exists >> but should not have been initialized fully. >> >> nbsp == NULL, nbrp == NULL >> >> It is necessary to check 'nbsp'. >> >> A router port's "peer", if set, must point to another router port, but the >> code as written also accepted switch ports. This caused problems when >> switch ports were actually specified. >> >> Reported-by: Gurucharan Shetty <g...@ovn.org> >> Reported-at: http://openvswitch.org/pipermail/dev/2016-July/075524.html >> Signed-off-by: Ben Pfaff <b...@ovn.org> >> Acked-by: Gurucharan Shetty <g...@ovn.org> >> Signed-off-by: nickcooper-zhangtonghao >> <nickcooper-zhangtong...@opencloud.tech> >> --- >> ovn/northd/ovn-northd.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c >> index a3d1672..efc915c 100644 >> --- a/ovn/northd/ovn-northd.c >> +++ b/ovn/northd/ovn-northd.c >> @@ -746,9 +746,15 @@ join_logical_ports(struct northd_context *ctx, >> } else if (op->nbrp && op->nbrp->peer) { >> struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer); >> if (peer) { >> + /* Deletion of a logical router port, which may be an other >> + * router peer, result in a WARN, because the "ports" >> variable >> + * still contains the logical port deleted. This port >> exists >> + * but should not have been initialized fully. >> + * >> + * nbsp == NULL, nbrp == NULL */ >> if (peer->nbrp) { >> op->peer = peer; >> - } else { >> + } else if (peer->nbsp) { >> /* An ovn_port for a switch port of type "router" does >> have >> * a router port as its peer (see the case above for >> * "router" ports), but this is set via >> options:router-port > > I don't understand this yet. I don't see how an ovn_port's nbsp and > nbrp can both be NULL. I only see two calls to ovn_port_create(), and > each of them supplies either nbsp or nbrp as nonnull. > > Can you explain further? > > Thanks, > > Ben.
Run the commands below, you will get a WARN info in the ovn-northd.log ovn-nbctl lr-add lr0 ovn-nbctl lr-add lr1 ovn-nbctl lrp-add lr0 lr0-p0 00:11:22:33:44:55 192.168.10.10/24 peer=lr1-p0 ovn-nbctl lrp-add lr1 lr1-p0 00:11:22:33:44:66 192.168.20.10/24 peer=lr0-p0 ovn-nbctl lrp-del lr1-p0 WARN INFO: “Bad configuration: The peer of router port lr0-p0 is a switch port.” Here’s is why: In the ‘join_logical_ports’ function, firstly, we get the ‘ovn_port’s from SB database and save them to “ports” variable. The ovn_port’s nbsp and nbrp both are NULL via ovn_port_create. Next, in the HMAP_FOR_EACH loop, we initialize the ‘ovn_port’s except which were deleted on the NB database. The ovn_port which is deleted in NB database is still in SB database. so, still in “ports” variable. Then, when connected to their peer, the ovn_port deleted may be found as a peer. But the ovn_port's nbsp and nbsp both are NULL. Thanks, zhangtonghao _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
