From patchwork Mon Sep 18 16:46:46 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xavier Simonart X-Patchwork-Id: 1836327 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=XoEvO9n/; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Rq9gv14xbz1ync for ; Tue, 19 Sep 2023 02:47:23 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 175A741B1B; Mon, 18 Sep 2023 16:47:21 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 175A741B1B Authentication-Results: smtp4.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=XoEvO9n/ X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ywJmn_WKxvdf; Mon, 18 Sep 2023 16:47:17 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp4.osuosl.org (Postfix) with ESMTPS id 001024194C; Mon, 18 Sep 2023 16:47:15 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 001024194C Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id D9DE5C0039; Mon, 18 Sep 2023 16:47:15 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) by lists.linuxfoundation.org (Postfix) with ESMTP id 152CBC0039 for ; Mon, 18 Sep 2023 16:47:14 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 72B5A404EF for ; Mon, 18 Sep 2023 16:46:54 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 72B5A404EF X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wVAo5P0nEuAj for ; Mon, 18 Sep 2023 16:46:52 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 15A6041936 for ; Mon, 18 Sep 2023 16:46:51 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 15A6041936 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1695055611; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=SqtrwHs4aVHETbyWHxOoXzCNUJ+KZFe3lSyquA37Cu8=; b=XoEvO9n/LpJsOYupDkfC0pNpM9Y7tIxJBgQETUJeOsdEdxXrAfuypxqe+V4yhKQi6YV7b4 2wooAmgwwWPBkFFn+TbPQFQB4OOR6Kn8WKxATjB74vadgGJRkkKS0vOt6QcwwxCyVYK8Rt E+hBlXjND9vyoaqkfZUUbuPH1zBC0/8= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-169-JZpL6DIOMjKIWYXh5S4DFA-1; Mon, 18 Sep 2023 12:46:47 -0400 X-MC-Unique: JZpL6DIOMjKIWYXh5S4DFA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 94C6C3800E94 for ; Mon, 18 Sep 2023 16:46:47 +0000 (UTC) Received: from wsfd-netdev90.ntdv.lab.eng.bos.redhat.com (wsfd-netdev90.ntdv.lab.eng.bos.redhat.com [10.19.188.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 76C72492C37; Mon, 18 Sep 2023 16:46:47 +0000 (UTC) From: Xavier Simonart To: xsimonar@redhat.com, dev@openvswitch.org Date: Mon, 18 Sep 2023 18:46:46 +0200 Message-Id: <20230918164647.3144917-2-xsimonar@redhat.com> In-Reply-To: <20230918164647.3144917-1-xsimonar@redhat.com> References: <20230918164647.3144917-1-xsimonar@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn 2/3] binding: handle pb->chassis and pb->up from if-status module X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 --- controller/binding.c | 122 ++++++++++++++++++++++++--------- controller/binding.h | 18 +++++ controller/if-status.c | 130 ++++++++++++++++++++++++++++-------- controller/if-status.h | 9 ++- controller/ovn-controller.c | 6 +- tests/ovn.at | 94 ++++++++++++++++++++++++++ 6 files changed, 318 insertions(+), 61 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index dbd2d52b7..fff631e7f 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -1200,7 +1200,7 @@ local_binding_set_pb(struct shash *local_bindings, const char *pb_name, * 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 +bool claimed_lport_set_up(const struct sbrec_port_binding *pb, const struct sbrec_port_binding *parent_pb, bool sb_readonly) @@ -1213,6 +1213,8 @@ claimed_lport_set_up(const struct sbrec_port_binding *pb, if (!parent_pb || (parent_pb->n_up && parent_pb->up[0])) { if (!sb_readonly) { if (pb->n_up) { + VLOG_INFO("Setting lport %s up in Southbound", + pb->logical_port); sbrec_port_binding_set_up(pb, &up, 1); } } else if (pb->n_up && !pb->up[0]) { @@ -1347,39 +1349,26 @@ 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, notify_up, + parent_pb); register_claim_timestamp(pb->logical_port, now); sset_find_and_delete(postponed_ports, pb->logical_port); } else { - 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]) || + (notify_up && !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, notify_up, + 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, notify_up, + parent_pb); update_tracked = true; } } @@ -2446,15 +2435,15 @@ 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); } + /* Clear the iface of the local binding. */ + lbinding->iface = NULL; + } remove_local_lports(iface_id, b_ctx_out); @@ -3247,7 +3236,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); } @@ -3464,6 +3453,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) { diff --git a/controller/binding.h b/controller/binding.h index 47df668a2..133badf3f 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -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'. @@ -258,4 +270,10 @@ void update_qos(struct ovsdb_idl_index * sbrec_port_binding_by_name, const struct ovsrec_open_vswitch_table *ovs_table, const struct ovsrec_bridge_table *bridge_table); +bool claimed_lport_set_up(const struct sbrec_port_binding *pb, + const struct sbrec_port_binding *parent_pb, + bool sb_readonly); +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 */ diff --git a/controller/if-status.c b/controller/if-status.c index 6c5afc866..a907dbbaa 100644 --- a/controller/if-status.c +++ b/controller/if-status.c @@ -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 notify_up; }; 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,22 @@ if_status_mgr_destroy(struct if_status_mgr *mgr) free(mgr); } +/* notify_up controls whether we wait for flows to be updated + * before setting the interface up. + * 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 notify_up, + 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 +301,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->notify_up = notify_up; 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 +376,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 +385,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 +420,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 +433,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_INFO("if_status_handle_claims for %s, notify_up = %d", iface->id, + iface->notify_up); + if (iface->notify_up) { + 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,18 +503,32 @@ 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) { - local_binding_set_pb(bindings, iface->id, chassis_rec, - NULL, true, iface->bind_type); - } else { - continue; + if (iface->notify_up) { + if (!local_bindings_pb_chassis_is_set(bindings, iface->id, + chassis_rec)) { + if (!sb_readonly) { + 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); + } + } 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); } - } - if (local_binding_is_up(bindings, iface->id, chassis_rec)) { - ovs_iface_set_state(mgr, iface, OIF_INSTALLED); } } @@ -528,9 +575,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->notify_up) { + 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]) { @@ -587,6 +638,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 = @@ -622,15 +674,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 { @@ -682,12 +737,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; } @@ -711,7 +772,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); } @@ -752,6 +814,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) { @@ -788,9 +851,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->notify_up) { + 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, sb_readonly); + } + } } free(ts_now_str); diff --git a/controller/if-status.h b/controller/if-status.h index 9714f6d8d..4ae5ad481 100644 --- a/controller/if-status.h +++ b/controller/if-status.h @@ -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, diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 859d9cab9..b09ac047e 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1815,6 +1815,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); @@ -1829,7 +1831,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; } @@ -5871,6 +5873,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()); diff --git a/tests/ovn.at b/tests/ovn.at index ba5ce298a..b4f146590 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -37004,3 +37004,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 +]) +