Originally, ovn_port_find_in_datapath() accepted an ovn_datapath and
port name as parameters and would search the ovn_datapath for an
ovn_port whose name matches the input port name's parameter.
In commit 0608e7089f34 ("northd: fix missing port up when deleting and
adding back an lsp"), the function was changed to accept a NB logical
switch port instead of a port name. This was done as a way to ensure
that the port that was found actually belonged to the NB logical switch
port in question.
There are three issues with this change.
1. Now there is no way to find an ovn_port by name. This is useful in
situations where a port is referred to by name, but we don't have a
NB logical switch port on hand. As an example, logical router
policies and static routes may refer to an output port by name. In
such a case, we should be able to find the port by name, since we
don't have the containing datapath.
2. This restricts the function to only finding ovn_ports representing
logical switch ports. We can't use it for finding ovn_ports
representing logical router ports.
3. This extra check is really only necessary when rebuilding ovn_ports
on an ovn_datapath during incremental processing. In any other
context, checking that the ovn_port's nbsp pointer matches the input
logical switch port is unnecessary.
For this commit, the original parameters for ovn_port_find_in_datapath()
have been restored. In ls_handle_lsp_changes(), we now check there that
the returned ovn_port's nbsp pointer matches the logical switch port
that we are inspecting. This commit also changes the ovn_datapath
parameter to be const, since it should have been in the first place.
Both the change to use a port name, as well as the change for the
ovn_datapath to be const will be important in upcoming commits.
Fixes: 0608e7089f34 ("northd: fix missing port up when deleting and
adding back an lsp")
Signed-off-by: Mark Michelson <[email protected]>
---
northd/northd.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 3f59894bd..4fb50fa19 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4758,13 +4758,12 @@ ls_port_has_changed(const struct
nbrec_logical_switch_port *new)
}
static struct ovn_port *
-ovn_port_find_in_datapath(struct ovn_datapath *od,
- const struct nbrec_logical_switch_port *nbsp)
+ovn_port_find_in_datapath(const struct ovn_datapath *od, const char *name)
{
struct ovn_port *op;
- HMAP_FOR_EACH_WITH_HASH (op, dp_node, hash_string(nbsp->name, 0),
+ HMAP_FOR_EACH_WITH_HASH (op, dp_node, hash_string(name, 0),
&od->ports) {
- if (!strcmp(op->key, nbsp->name) && op->nbsp == nbsp) {
+ if (!strcmp(op->key, name)) {
return op;
}
}
@@ -5005,13 +5004,18 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
{
bool ls_deleted = nbrec_logical_switch_is_deleted(changed_ls);
bool ls_ports_changed = ls_deleted;
+ struct ovn_port *op;
if (!nbrec_logical_switch_is_updated(changed_ls,
NBREC_LOGICAL_SWITCH_COL_PORTS)) {
for (size_t i = 0; i < changed_ls->n_ports; i++) {
if (nbrec_logical_switch_port_row_get_seqno(
- changed_ls->ports[i], OVSDB_IDL_CHANGE_MODIFY) > 0 ||
- !ovn_port_find_in_datapath(od, changed_ls->ports[i])) {
+ changed_ls->ports[i], OVSDB_IDL_CHANGE_MODIFY) > 0) {
+ ls_ports_changed = true;
+ break;
+ }
+ op = ovn_port_find_in_datapath(od, changed_ls->ports[i]->name);
+ if (!op || op->nbsp != changed_ls->ports[i]) {
ls_ports_changed = true;
break;
}
@@ -5028,7 +5032,6 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
&& (vector_len(&od->router_ports) == hmap_count(&od->ports)));
struct ovs_list existing_virtual_ports;
- struct ovn_port *op;
ovs_list_init(&existing_virtual_ports);
HMAP_FOR_EACH (op, dp_node, &od->ports) {
@@ -5040,9 +5043,9 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn *ovnsb_idl_txn,
/* Compare the individual ports in the old and new Logical Switches */
for (size_t j = 0; j < changed_ls->n_ports; ++j) {
struct nbrec_logical_switch_port *new_nbsp = changed_ls->ports[j];
- op = ovn_port_find_in_datapath(od, new_nbsp);
+ op = ovn_port_find_in_datapath(od, new_nbsp->name);
- if (!op) {
+ if (!op || op->nbsp != new_nbsp) {
if (!lsp_can_be_inc_processed(new_nbsp)) {
goto fail;
}
--
2.55.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
