@@ -1193,33 +1193,22 @@ local_binding_set_pb(struct shash *local_bindings, const char *pb_name,
* - set the 'pb.up' field to true if 'parent_pb.up' is 'true' (e.g., for
* container and virtual ports).
*
- * Returns false if lport is not claimed due to 'sb_readonly'.
- * Returns true otherwise.
- *
* Note:
* Updates the 'pb->up' field only if it's explicitly set to 'false'.
* This is to ensure compatibility with older versions of ovn-northd.
*/
-static bool
+void
claimed_lport_set_up(const struct sbrec_port_binding *pb,
- const struct sbrec_port_binding *parent_pb,
- bool sb_readonly)
+ const struct sbrec_port_binding *parent_pb)
{
- /* When notify_up is false in claim_port(), no state is created
- * by if_status_mgr. In such cases, return false (i.e. trigger recompute)
- * if we can't update sb (because it is readonly).
- */
bool up = true;
if (!parent_pb || (parent_pb->n_up && parent_pb->up[0])) {
- if (!sb_readonly) {
- if (pb->n_up) {
- sbrec_port_binding_set_up(pb, &up, 1);
- }
- } else if (pb->n_up && !pb->up[0]) {
- return false;
+ if (pb->n_up) {
+ VLOG_INFO("Setting lport %s up in Southbound",
+ pb->logical_port);
+ sbrec_port_binding_set_up(pb, &up, 1);
}
}
- return true;
}
typedef void (*set_func)(const struct sbrec_port_binding *pb,
@@ -1322,7 +1311,7 @@ claim_lport(const struct sbrec_port_binding *pb,
const struct sbrec_port_binding *parent_pb,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface *iface_rec,
- bool sb_readonly, bool notify_up,
+ bool sb_readonly, bool is_vif,
struct hmap *tracked_datapaths,
struct if_status_mgr *if_mgr,
struct sset *postponed_ports)
@@ -1347,40 +1336,27 @@ claim_lport(const struct sbrec_port_binding *pb,
}
update_tracked = true;
- if (!notify_up) {
- if (!claimed_lport_set_up(pb, parent_pb, sb_readonly)) {
- return false;
- }
- if (sb_readonly) {
- return false;
- }
- set_pb_chassis_in_sbrec(pb, chassis_rec, true);
- } else {
- if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec,
- sb_readonly, can_bind);
- }
+ if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec,
+ sb_readonly, can_bind, is_vif,
+ parent_pb);
register_claim_timestamp(pb->logical_port, now);
sset_find_and_delete(postponed_ports, pb->logical_port);
} else {
update_tracked = true;
- if (!notify_up) {
- if (!claimed_lport_set_up(pb, parent_pb, sb_readonly)) {
- return false;
- }
- } else {
- if ((pb->n_up && !pb->up[0]) ||
- !smap_get_bool(&iface_rec->external_ids,
- OVN_INSTALLED_EXT_ID, false)) {
- if_status_mgr_claim_iface(if_mgr, pb, chassis_rec,
- iface_rec, sb_readonly,
- can_bind);
- }
+ if ((pb->n_up && !pb->up[0]) ||
+ (is_vif && !smap_get_bool(&iface_rec->external_ids,
+ OVN_INSTALLED_EXT_ID, false))) {
+ if_status_mgr_claim_iface(if_mgr, pb, chassis_rec,
+ iface_rec, sb_readonly,
+ can_bind, is_vif,
+ parent_pb);
}
}
} else if (can_bind == CAN_BIND_AS_ADDITIONAL) {
if (!is_additional_chassis(pb, chassis_rec)) {
if_status_mgr_claim_iface(if_mgr, pb, chassis_rec, iface_rec,
- sb_readonly, can_bind);
+ sb_readonly, can_bind, is_vif,
+ parent_pb);
update_tracked = true;
}
}
@@ -2447,14 +2423,14 @@ consider_iface_release(const struct ovsrec_interface *iface_rec,
remove_related_lport(b_lport->pb, b_ctx_out);
}
- /* Clear the iface of the local binding. */
- lbinding->iface = NULL;
-
/* Check if the lbinding has children of type PB_CONTAINER.
* If so, don't delete the local_binding. */
if (!is_lbinding_container_parent(lbinding)) {
local_binding_delete(lbinding, local_bindings, binding_lports,
b_ctx_out->if_mgr);
+ } else {
+ /* Clear the iface of the local binding. */
+ lbinding->iface = NULL;
}
}
@@ -3248,7 +3224,7 @@ local_binding_delete(struct local_binding *lbinding,
struct if_status_mgr *if_mgr)
{
shash_find_and_delete(local_bindings, lbinding->name);
- if_status_mgr_delete_iface(if_mgr, lbinding->name);
+ if_status_mgr_delete_iface(if_mgr, lbinding->name, lbinding->iface);
local_binding_destroy(lbinding, binding_lports);
}
@@ -3465,6 +3441,79 @@ port_binding_set_down(const struct sbrec_chassis *chassis_rec,
}
}
+void
+port_binding_set_up(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const char *iface_id,
+ const struct uuid *pb_uuid)
+{
+ const struct sbrec_port_binding *pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table, pb_uuid);
+ if (!pb) {
+ VLOG_DBG("port_binding already deleted for %s", iface_id);
+ return;
+ }
+ if (pb->n_up && !pb->up[0]) {
+ bool up = true;
+ sbrec_port_binding_set_up(pb, &up, 1);
+ VLOG_INFO("Setting lport %s up in Southbound", pb->logical_port);
+ set_pb_chassis_in_sbrec(pb, chassis_rec, true);
+ }
+}
+
+void
+port_binding_set_pb(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const char *iface_id,
+ const struct uuid *pb_uuid,
+ enum can_bind bind_type)
+{
+ const struct sbrec_port_binding *pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table, pb_uuid);
+ if (!pb) {
+ VLOG_DBG("port_binding already deleted for %s", iface_id);
+ return;
+ }
+ if (bind_type == CAN_BIND_AS_MAIN) {
+ set_pb_chassis_in_sbrec(pb, chassis_rec, true);
+ } else if (bind_type == CAN_BIND_AS_ADDITIONAL) {
+ set_pb_additional_chassis_in_sbrec(pb, chassis_rec, true);
+ }
+}
+
+bool
+port_binding_pb_chassis_is_set(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const struct uuid *pb_uuid)
+{
+ const struct sbrec_port_binding *pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table, pb_uuid);
+
+ if (pb && ((pb->chassis == chassis_rec) ||
+ is_additional_chassis(pb, chassis_rec))) {
+ return true;
+ }
+ return false;
+}
+
+bool
+port_binding_is_up(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const struct uuid *pb_uuid)
+{
+ const struct sbrec_port_binding *pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table, pb_uuid);
+
+ if (!pb || pb->chassis != chassis_rec) {
+ return false;
+ }
+
+ if (pb->n_up && !pb->up[0]) {
+ return false;
+ }
+ return true;
+}
+
static void
binding_lport_set_up(struct binding_lport *b_lport, bool sb_readonly)
{
@@ -217,6 +217,18 @@ void port_binding_set_down(const struct sbrec_chassis *chassis_rec,
const struct sbrec_port_binding_table *pb_table,
const char *iface_id,
const struct uuid *pb_uuid);
+void port_binding_set_up(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const char *iface_id,
+ const struct uuid *pb_uuid);
+void port_binding_set_pb(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const char *iface_id,
+ const struct uuid *pb_uuid,
+ enum can_bind bind_type);
+bool port_binding_pb_chassis_is_set(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const struct uuid *pb_uuid);
/* This structure represents a logical port (or port binding)
* which is associated with 'struct local_binding'.
@@ -261,4 +273,9 @@ void update_qos(struct ovsdb_idl_index * sbrec_port_binding_by_name,
bool lport_maybe_postpone(const char *port_name, long long int now,
struct sset *postponed_ports);
+void claimed_lport_set_up(const struct sbrec_port_binding *pb,
+ const struct sbrec_port_binding *parent_pb);
+bool port_binding_is_up(const struct sbrec_chassis *chassis_rec,
+ const struct sbrec_port_binding_table *pb_table,
+ const struct uuid *pb_uuid);
#endif /* controller/binding.h */
@@ -177,7 +177,9 @@ static const char *if_state_names[] = {
struct ovs_iface {
char *id; /* Extracted from OVS external_ids.iface_id. */
+ char *name; /* OVS iface name. */
struct uuid pb_uuid; /* Port_binding uuid */
+ struct uuid parent_pb_uuid; /* Port_binding uuid */
enum if_state state; /* State of the interface in the state machine. */
uint32_t install_seqno; /* Seqno at which this interface is expected to
* be fully programmed in OVS. Only used in state
@@ -185,6 +187,7 @@ struct ovs_iface {
*/
uint16_t mtu; /* Extracted from OVS interface.mtu field. */
enum can_bind bind_type;/* CAN_BIND_AS_MAIN or CAN_BIND_AS_ADDITIONAL */
+ bool is_vif; /* Vifs, container or virtual ports */
};
static uint64_t ifaces_usage;
@@ -224,6 +227,7 @@ static void if_status_mgr_update_bindings(
struct if_status_mgr *mgr, struct local_binding_data *binding_data,
const struct sbrec_chassis *,
const struct ovsrec_interface_table *iface_table,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly, bool ovs_readonly);
static void ovn_uninstall_hash_account_mem(const char *name, bool erase);
@@ -273,12 +277,23 @@ if_status_mgr_destroy(struct if_status_mgr *mgr)
free(mgr);
}
+/* is_vif controls whether we wait for flows to be updated
+ * before setting the interface up. It is true for VIF, CONTAINER
+ * and VIRTUAL ports.
+ * Non-VIF ports are reported up as soon as they are claimed
+ * to maintain compatibility with older versions.
+ * See aae25e6 binding: Correctly set Port_Binding.up for container/virtual
+ * ports.
+ */
+
void
if_status_mgr_claim_iface(struct if_status_mgr *mgr,
const struct sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface *iface_rec,
- bool sb_readonly, enum can_bind bind_type)
+ bool sb_readonly, enum can_bind bind_type,
+ bool is_vif,
+ const struct sbrec_port_binding *parent_pb)
{
const char *iface_id = pb->logical_port;
struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
@@ -287,8 +302,13 @@ if_status_mgr_claim_iface(struct if_status_mgr *mgr,
iface = ovs_iface_create(mgr, iface_id, iface_rec, OIF_CLAIMED);
}
iface->bind_type = bind_type;
+ iface->is_vif = is_vif;
memcpy(&iface->pb_uuid, &pb->header_.uuid, sizeof(iface->pb_uuid));
+ if (parent_pb) {
+ memcpy(&iface->parent_pb_uuid, &parent_pb->header_.uuid,
+ sizeof(iface->pb_uuid));
+ }
if (!sb_readonly) {
if (bind_type == CAN_BIND_AS_MAIN) {
set_pb_chassis_in_sbrec(pb, chassis_rec, true);
@@ -357,7 +377,8 @@ if_status_mgr_release_iface(struct if_status_mgr *mgr, const char *iface_id)
}
void
-if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id)
+if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id,
+ const struct ovsrec_interface *iface_rec)
{
struct ovs_iface *iface = shash_find_data(&mgr->ifaces, iface_id);
@@ -365,6 +386,12 @@ if_status_mgr_delete_iface(struct if_status_mgr *mgr, const char *iface_id)
return;
}
+ if (iface_rec && strcmp(iface->name, iface_rec->name)) {
+ VLOG_DBG("Interface %s not deleted as port %s bound to %s",
+ iface_rec->name, iface_id, iface->name);
+ return;
+ }
+
switch (iface->state) {
case OIF_CLAIMED:
case OIF_INSTALL_FLOWS:
@@ -394,6 +421,7 @@ if_status_handle_claims(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
struct hmap *tracked_datapath,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly)
{
if (!binding_data || sb_readonly) {
@@ -406,9 +434,15 @@ if_status_handle_claims(struct if_status_mgr *mgr,
bool rc = false;
HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_CLAIMED]) {
struct ovs_iface *iface = node->data;
- VLOG_INFO("if_status_handle_claims for %s", iface->id);
- local_binding_set_pb(bindings, iface->id, chassis_rec,
- tracked_datapath, true, iface->bind_type);
+ VLOG_DBG("if_status_handle_claims for %s, is_vif = %d", iface->id,
+ iface->is_vif);
+ if (iface->is_vif) {
+ local_binding_set_pb(bindings, iface->id, chassis_rec,
+ tracked_datapath, true, iface->bind_type);
+ } else {
+ port_binding_set_pb(chassis_rec, pb_table, iface->id,
+ &iface->pb_uuid, iface->bind_type);
+ }
rc = true;
}
return rc;
@@ -470,23 +504,37 @@ if_status_mgr_update(struct if_status_mgr *mgr,
*/
HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_MARK_UP]) {
struct ovs_iface *iface = node->data;
-
- if (!local_bindings_pb_chassis_is_set(bindings, iface->id,
- chassis_rec)) {
- if (!sb_readonly) {
- long long int now = time_msec();
- if (lport_maybe_postpone(iface->id, now,
- get_postponed_ports())) {
+ if (iface->is_vif) {
+ if (!local_bindings_pb_chassis_is_set(bindings, iface->id,
+ chassis_rec)) {
+ if (!sb_readonly) {
+ long long int now = time_msec();
+ if (lport_maybe_postpone(iface->id, now,
+ get_postponed_ports())) {
+ continue;
+ }
+ local_binding_set_pb(bindings, iface->id, chassis_rec,
+ NULL, true, iface->bind_type);
+ } else {
continue;
}
- local_binding_set_pb(bindings, iface->id, chassis_rec,
- NULL, true, iface->bind_type);
- } else {
- continue;
}
- }
- if (local_binding_is_up(bindings, iface->id, chassis_rec)) {
- ovs_iface_set_state(mgr, iface, OIF_INSTALLED);
+ if (local_binding_is_up(bindings, iface->id, chassis_rec)) {
+ ovs_iface_set_state(mgr, iface, OIF_INSTALLED);
+ }
+ } else {
+ if (!port_binding_pb_chassis_is_set(chassis_rec, pb_table,
+ &iface->pb_uuid)) {
+ if (!sb_readonly) {
+ port_binding_set_pb(chassis_rec, pb_table, iface->id,
+ &iface->pb_uuid, iface->bind_type);
+ } else {
+ continue;
+ }
+ }
+ if (port_binding_is_up(chassis_rec, pb_table, &iface->pb_uuid)) {
+ ovs_iface_set_state(mgr, iface, OIF_INSTALLED);
+ }
}
}
@@ -537,9 +585,13 @@ if_status_mgr_update(struct if_status_mgr *mgr,
/* No need to to update pb->chassis as already done
* in if_status_handle_claims or if_status_mgr_claim_iface
*/
- ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS);
- iface->install_seqno = mgr->iface_seqno + 1;
- new_ifaces = true;
+ if (iface->is_vif) {
+ ovs_iface_set_state(mgr, iface, OIF_INSTALL_FLOWS);
+ iface->install_seqno = mgr->iface_seqno + 1;
+ new_ifaces = true;
+ } else {
+ ovs_iface_set_state(mgr, iface, OIF_MARK_UP);
+ }
}
} else {
HMAPX_FOR_EACH_SAFE (node, &mgr->ifaces_per_state[OIF_CLAIMED]) {
@@ -596,6 +648,7 @@ if_status_mgr_run(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface_table *iface_table,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly, bool ovs_readonly)
{
struct ofctrl_acked_seqnos *acked_seqnos =
@@ -631,15 +684,18 @@ if_status_mgr_run(struct if_status_mgr *mgr,
/* Update binding states. */
if_status_mgr_update_bindings(mgr, binding_data, chassis_rec,
- iface_table,
+ iface_table, pb_table,
sb_readonly, ovs_readonly);
}
static void
-ovs_iface_account_mem(const char *iface_id, bool erase)
+ovs_iface_account_mem(const char *iface_id, char *iface_name, bool erase)
{
uint32_t size = (strlen(iface_id) + sizeof(struct ovs_iface) +
sizeof(struct shash_node));
+ if (iface_name) {
+ size += strlen(iface_name);
+ }
if (erase) {
ifaces_usage -= size;
} else {
@@ -691,12 +747,18 @@ ovs_iface_create(struct if_status_mgr *mgr, const char *iface_id,
{
struct ovs_iface *iface = xzalloc(sizeof *iface);
- VLOG_DBG("Interface %s create.", iface_id);
+ VLOG_DBG("Interface %s create for iface %s.", iface_id,
+ iface_rec ? iface_rec->name : "");
iface->id = xstrdup(iface_id);
shash_add_nocopy(&mgr->ifaces, iface->id, iface);
ovs_iface_set_state(mgr, iface, state);
- ovs_iface_account_mem(iface_id, false);
- if_status_mgr_iface_update(mgr, iface_rec);
+ if (iface_rec) {
+ ovs_iface_account_mem(iface_id, iface_rec->name, false);
+ if_status_mgr_iface_update(mgr, iface_rec);
+ iface->name = xstrdup(iface_rec->name);
+ } else {
+ ovs_iface_account_mem(iface_id, NULL, false);
+ }
return iface;
}
@@ -720,7 +782,8 @@ ovs_iface_destroy(struct if_status_mgr *mgr, struct ovs_iface *iface)
if (node) {
shash_steal(&mgr->ifaces, node);
}
- ovs_iface_account_mem(iface->id, true);
+ ovs_iface_account_mem(iface->id, iface->name, true);
+ free(iface->name);
free(iface->id);
free(iface);
}
@@ -761,6 +824,7 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface_table *iface_table,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly, bool ovs_readonly)
{
if (!binding_data) {
@@ -797,9 +861,20 @@ if_status_mgr_update_bindings(struct if_status_mgr *mgr,
char *ts_now_str = xasprintf("%lld", time_wall_msec());
HMAPX_FOR_EACH (node, &mgr->ifaces_per_state[OIF_MARK_UP]) {
struct ovs_iface *iface = node->data;
-
- local_binding_set_up(bindings, iface->id, chassis_rec, ts_now_str,
- sb_readonly, ovs_readonly);
+ if (iface->is_vif) {
+ local_binding_set_up(bindings, iface->id, chassis_rec, ts_now_str,
+ sb_readonly, ovs_readonly);
+ } else if (!sb_readonly) {
+ const struct sbrec_port_binding *pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table,
+ &iface->pb_uuid);
+ if (pb) {
+ const struct sbrec_port_binding *parent_pb =
+ sbrec_port_binding_table_get_for_uuid(pb_table,
+ &iface->parent_pb_uuid);
+ claimed_lport_set_up(pb, parent_pb);
+ }
+ }
}
free(ts_now_str);
@@ -32,9 +32,12 @@ void if_status_mgr_claim_iface(struct if_status_mgr *,
const struct sbrec_port_binding *pb,
const struct sbrec_chassis *chassis_rec,
const struct ovsrec_interface *iface_rec,
- bool sb_readonly, enum can_bind bind_type);
+ bool sb_readonly, enum can_bind bind_type,
+ bool notify_up,
+ const struct sbrec_port_binding *parent_pb);
void if_status_mgr_release_iface(struct if_status_mgr *, const char *iface_id);
-void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id);
+void if_status_mgr_delete_iface(struct if_status_mgr *, const char *iface_id,
+ const struct ovsrec_interface *iface_rec);
void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data *,
const struct sbrec_chassis *chassis,
@@ -45,6 +48,7 @@ void if_status_mgr_update(struct if_status_mgr *, struct local_binding_data *,
void if_status_mgr_run(struct if_status_mgr *mgr, struct local_binding_data *,
const struct sbrec_chassis *,
const struct ovsrec_interface_table *iface_table,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly, bool ovs_readonly);
void if_status_mgr_get_memory_usage(struct if_status_mgr *mgr,
struct simap *usage);
@@ -54,6 +58,7 @@ bool if_status_handle_claims(struct if_status_mgr *mgr,
struct local_binding_data *binding_data,
const struct sbrec_chassis *chassis_rec,
struct hmap *tracked_datapath,
+ const struct sbrec_port_binding_table *pb_table,
bool sb_readonly);
void if_status_mgr_remove_ovn_installed(struct if_status_mgr *mgr,
const char *name,
@@ -1820,6 +1820,8 @@ runtime_data_sb_ro_handler(struct engine_node *node, void *data)
engine_ovsdb_node_get_index(
engine_get_input("SB_chassis", node),
"name");
+ const struct sbrec_port_binding_table *pb_table =
+ EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
if (chassis_id) {
chassis = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
@@ -1834,7 +1836,7 @@ runtime_data_sb_ro_handler(struct engine_node *node, void *data)
&rt_data->lbinding_data,
chassis,
&rt_data->tracked_dp_bindings,
- sb_readonly)) {
+ pb_table, sb_readonly)) {
engine_set_node_state(node, EN_UPDATED);
rt_data->tracked = true;
}
@@ -5880,6 +5882,8 @@ main(int argc, char *argv[])
if_status_mgr_run(if_mgr, binding_data, chassis,
ovsrec_interface_table_get(
ovs_idl_loop.idl),
+ sbrec_port_binding_table_get(
+ ovnsb_idl_loop.idl),
!ovnsb_idl_txn, !ovs_idl_txn);
stopwatch_stop(IF_STATUS_MGR_RUN_STOPWATCH_NAME,
time_msec());
@@ -36957,3 +36957,97 @@ AT_CHECK([grep -c "NXT_CT_FLUSH_ZONE" hv1/ovs-vswitchd.log], [0], [dnl
OVN_CLEANUP([hv1])
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([virtual port claim race condition])
+AT_KEYWORDS([virtual ports])
+ovn_start
+
+send_garp() {
+ local hv=$1 inport=$2 eth_src=$3 eth_dst=$4 spa=$5 tpa=$6
+ local request=${eth_dst}${eth_src}08060001080006040001${eth_src}${spa}${eth_dst}${tpa}
+ as hv$hv ovs-appctl netdev-dummy/receive hv${hv}-vif$inport $request
+}
+
+net_add n1
+
+sim_add hv1
+as hv1
+ovs-vsctl add-br br-phys
+ovn_attach n1 br-phys 192.168.0.1
+ovn-appctl vlog/set dbg
+ovs-vsctl -- add-port br-int hv1-vif1 -- \
+ set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
+ options:tx_pcap=hv1/vif1-tx.pcap \
+ options:rxq_pcap=hv1/vif1-rx.pcap \
+ ofport-request=1
+ovs-vsctl -- add-port br-int hv1-vif3 -- \
+ set interface hv1-vif3 \
+ options:tx_pcap=hv1/vif3-tx.pcap \
+ options:rxq_pcap=hv1/vif3-rx.pcap \
+ ofport-request=3
+
+ovs-appctl -t ovn-controller vlog/set dbg
+
+ovn-nbctl ls-add sw0
+
+check ovn-nbctl lsp-add sw0 sw0-vir
+check ovn-nbctl lsp-set-addresses sw0-vir "50:54:00:00:00:10 10.0.0.10"
+check ovn-nbctl lsp-set-port-security sw0-vir "50:54:00:00:00:10 10.0.0.10"
+check ovn-nbctl lsp-set-type sw0-vir virtual
+check ovn-nbctl set logical_switch_port sw0-vir options:virtual-ip=10.0.0.10
+check ovn-nbctl set logical_switch_port sw0-vir options:virtual-parents=sw0-p1
+
+check ovn-nbctl lsp-add sw0 sw0-p1
+check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:00:00:00:03 10.0.0.3 1000::3"
+check ovn-nbctl lsp-set-port-security sw0-p1 "50:54:00:00:00:03 10.0.0.3 10.0.0.10 1000::3"
+
+# Create a logical router and attach both logical switches
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 1000::a/64
+check ovn-nbctl lsp-add sw0 sw0-lr0
+check ovn-nbctl lsp-set-type sw0-lr0 router
+check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
+check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
+
+wait_for_ports_up
+ovn-nbctl --wait=hv sync
+hv1_ch_uuid=`ovn-sbctl --bare --columns _uuid find chassis name="hv1"`
+
+# Try to bind sw0-vir directly to an OVS port. This should be ignored by
+# ovn-controller.
+as hv1 ovs-vsctl set interface hv1-vif3 external-ids:iface-id=sw0-vir
+
+# Make sb to sleep, so that claim of sw0-vir (through pinctrl) and hv1-vif3 can be handled within same idl by controller
+sleep_sb
+
+# From sw0-p0 send GARP for 10.0.0.10. hv1 should claim sw0-vir
+# and sw0-p1 should be its virtual_parent.
+eth_src=505400000003
+eth_dst=ffffffffffff
+spa=$(ip_to_hex 10 0 0 10)
+tpa=$(ip_to_hex 10 0 0 10)
+send_garp 1 1 $eth_src $eth_dst $spa $tpa
+
+OVS_WAIT_UNTIL([test 1 = `cat hv1/ovn-controller.log | grep "pinctrl received packet-in" | \
+grep opcode=BIND_VPORT | grep OF_Table_ID=29 | wc -l`])
+
+sleep_controller hv1
+
+# Cleanup hv1-vif3. This should not interfere with sw0-vir claim
+as hv1 ovs-vsctl del-port hv1-vif3
+
+wake_up_sb
+ovn-nbctl --wait=sb sync
+wake_up_controller hv1
+check ovn-nbctl --wait=hv sync
+
+wait_row_count Port_Binding 1 logical_port=sw0-vir chassis=$hv1_ch_uuid
+check_row_count Port_Binding 1 logical_port=sw0-vir virtual_parent=sw0-p1
+wait_for_ports_up sw0-vir
+check ovn-nbctl --wait=hv sync
+
+OVN_CLEANUP([hv1])
+AT_CLEANUP
+])
+
Before this patch, if-status module handled pb->chassis and pb->up for vif ports, but not for non-VIF ports. This caused a few potential issues: - It was difficult to check that a no-vif port has been previously claimed as there was no state in if-status. Hence it was difficult to always release such a port when pb->chassis was set to a different chassis. This issue will be fixed in a future patch. - Releasing such ports caused ovn-controller to log warnings such as "Trying to delete unknown ports". - If sb is readonly at the time of the claim, it forced the module to recompute. This patch fixed issues two and three by moving the work of setting pb->chassis and pb->up to the if-status module. Signed-off-by: Xavier Simonart <xsimonar@redhat.com> --- v2: Avoid clearing iface if already deleted v3: - handled Mark's feedback i.e. - clear comment in claimed_lport_set_up - changed notify_up to is_vif and clarify comments - change claimed_lport_set_up to void as was always called with !sb_readonly --- controller/binding.c | 143 ++++++++++++++++++++++++------------ controller/binding.h | 17 +++++ controller/if-status.c | 137 ++++++++++++++++++++++++++-------- controller/if-status.h | 9 ++- controller/ovn-controller.c | 6 +- tests/ovn.at | 94 ++++++++++++++++++++++++ 6 files changed, 325 insertions(+), 81 deletions(-)