From patchwork Thu Apr 1 23:20:53 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 1461446 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4FBK3l0zznz9sV5 for ; Fri, 2 Apr 2021 10:22:59 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 498C160D75; Thu, 1 Apr 2021 23:22:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3eyGmo6KDARl; Thu, 1 Apr 2021 23:22:07 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp3.osuosl.org (Postfix) with ESMTP id 9525960DB0; Thu, 1 Apr 2021 23:22:00 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 78518C000B; Thu, 1 Apr 2021 23:22:00 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1F264C0015 for ; Thu, 1 Apr 2021 23:21:59 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 1FB0F60D6A for ; Thu, 1 Apr 2021 23:21:39 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id l3-A6LCJW3TV for ; Thu, 1 Apr 2021 23:21:37 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp3.osuosl.org (Postfix) with ESMTPS id 1E29360C20 for ; Thu, 1 Apr 2021 23:21:34 +0000 (UTC) X-Originating-IP: 75.54.222.30 Received: from sigfpe.attlocal.net (75-54-222-30.lightspeed.rdcyca.sbcglobal.net [75.54.222.30]) (Authenticated sender: blp@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 656951C0008; Thu, 1 Apr 2021 23:21:32 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Thu, 1 Apr 2021 16:20:53 -0700 Message-Id: <20210401232108.3902274-12-blp@ovn.org> X-Mailer: git-send-email 2.29.2 In-Reply-To: <20210401232108.3902274-1-blp@ovn.org> References: <20210401232108.3902274-1-blp@ovn.org> MIME-Version: 1.0 Cc: Ben Pfaff , Dumitru Ceara Subject: [ovs-dev] [PATCH v2 11/26] ovn-northd-ddlog: Preserve NB_Global more carefully. 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" Dumitru reported in #openvswitch that ovn-northd-ddlog can discard the setting of NB_Global.options:use_logical_dp_groups at startup. I think that this must be because it seems possible that at startup some of the relations in the Out_NB_Global rule aren't populated yet, and yet there is still a row in nb::NB_Global, so that neither rule for Out_NB_Global matches and therefore ovn-northd-ddlog deletes the row. This commit changes the structure of how ovn-northd-ddlog generates Out_NB_Global to ensure that, if there's an input row, it always generates exactly one output row. This should be more robust than the previous version regardless of whether it fixes the exact problem that Dumitru observed (which I did not try to reproduce). Reported-by: Dumitru Ceara Signed-off-by: Ben Pfaff --- northd/ovn_northd.dl | 63 ++++++++++++++++++++++++++------------------ 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 0063021e13f5..d718425b7de3 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -625,21 +625,25 @@ HvCfgTimestamp(hv_cfg_timestamp) :- not HvCfgTimestamp0(). /* - * NB_Global: - * - set `sb_cfg` to the value of `SB_Global.nb_cfg`. - * - set `hv_cfg` to the smallest value of `nb_cfg` across all `Chassis` - * - FIXME: we use ipsec as unique key to make sure that we don't create multiple `NB_Global` - * instance. There is a potential race condition if this field is modified at the same - * time northd is updating `sb_cfg` or `hv_cfg`. + * nb::Out_NB_Global. + * + * OutNBGlobal0 generates the new record in the common case. + * OutNBGlobal1 generates the new record as a copy of nb::NB_Global, if sb::SB_Global is missing. + * nb::Out_NB_Global makes sure we have only a single record in the relation. + * + * (We don't generate an NB_Global output record if there isn't + * one in the input. We don't have enough entropy available to + * generate a random _uuid. Doesn't seem like a big deal, because + * OVN probably hasn't really been initialized yet.) */ -input relation NbCfgTimestamp[integer] -nb::Out_NB_Global(._uuid = _uuid, - .sb_cfg = sb_cfg, - .hv_cfg = hv_cfg, - .nb_cfg_timestamp = nb_cfg_timestamp, - .hv_cfg_timestamp = hv_cfg_timestamp, - .ipsec = ipsec, - .options = options) :- +relation OutNBGlobal0[nb::Out_NB_Global] +OutNBGlobal0[nb::Out_NB_Global{._uuid = _uuid, + .sb_cfg = sb_cfg, + .hv_cfg = hv_cfg, + .nb_cfg_timestamp = nb_cfg_timestamp, + .hv_cfg_timestamp = hv_cfg_timestamp, + .ipsec = ipsec, + .options = options}] :- NbCfgTimestamp[nb_cfg_timestamp], HvCfgTimestamp(hv_cfg_timestamp), nbg in nb::NB_Global(._uuid = _uuid, .ipsec = ipsec), @@ -654,19 +658,26 @@ nb::Out_NB_Global(._uuid = _uuid, var options2 = options1.insert_imm("max_tunid", "${max_tunid}"), var options = options2.insert_imm("northd_internal_version", ovn_internal_version()). +relation OutNBGlobal1[nb::Out_NB_Global] +OutNBGlobal1[x] :- OutNBGlobal0[x]. +OutNBGlobal1[nb::Out_NB_Global{._uuid = nbg._uuid, + .sb_cfg = nbg.sb_cfg, + .hv_cfg = nbg.hv_cfg, + .ipsec = nbg.ipsec, + .options = nbg.options, + .nb_cfg_timestamp = nbg.nb_cfg_timestamp, + .hv_cfg_timestamp = nbg.hv_cfg_timestamp}] :- + Unit(), + not OutNBGlobal0[_], + nbg in nb::NB_Global(). + +nb::Out_NB_Global[y] :- + OutNBGlobal1[x], + var y = x.group_by(()).group_first(). -/* SB_Global does not exist yet -- just keep the old value of NB_Global */ -nb::Out_NB_Global(._uuid = nbg._uuid, - .sb_cfg = nbg.sb_cfg, - .hv_cfg = nbg.hv_cfg, - .ipsec = nbg.ipsec, - .options = nbg.options, - .nb_cfg_timestamp = nb_cfg_timestamp, - .hv_cfg_timestamp = hv_cfg_timestamp) :- - NbCfgTimestamp[nb_cfg_timestamp], - HvCfgTimestamp(hv_cfg_timestamp), - nbg in nb::NB_Global(), - not sb::SB_Global(). +// Tracks the value that should go into NB_Global's 'nb_cfg_timestamp' column. +// ovn-northd-ddlog.c pushes the current time directly into this relation. +input relation NbCfgTimestamp[integer] output relation SbCfg[integer] SbCfg[sb_cfg] :- nb::Out_NB_Global(.sb_cfg = sb_cfg).