From patchwork Mon Jun 21 06:51:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1494918 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=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 4G7gFR6yhPz9s5R for ; Mon, 21 Jun 2021 16:52:27 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 46AB84037A; Mon, 21 Jun 2021 06:52:24 +0000 (UTC) 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 faTgHB-VusXN; Mon, 21 Jun 2021 06:52:20 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 49CA140408; Mon, 21 Jun 2021 06:52:09 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 748F5C000C; Mon, 21 Jun 2021 06:52:07 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 0D76DC000E for ; Mon, 21 Jun 2021 06:52:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id DE41A400ED for ; Mon, 21 Jun 2021 06:52:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id qU6niRii5CYI for ; Mon, 21 Jun 2021 06:52:02 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp2.osuosl.org (Postfix) with ESMTPS id EFFF5400CD for ; Mon, 21 Jun 2021 06:52:01 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 2906BE0004; Mon, 21 Jun 2021 06:51:58 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Sun, 20 Jun 2021 23:51:33 -0700 Message-Id: <20210621065135.1236639-2-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621065135.1236639-1-hzhou@ovn.org> References: <20210621065135.1236639-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 1/3] ovn-northd: Remove lflow_add_unique. 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" This patch removes the workaround when adding multicast group related lflows, because the multicast group dependency problem is fixed in ovn-controller in the previous commit. This patch also removes the UniqueFlow/AnnotatedFlow usage in northd DDlog implementation for the same reason. Signed-off-by: Han Zhou Acked-by: Dumitru Ceara --- northd/ovn-northd.c | 93 +++++++---------- northd/ovn_northd.dl | 232 ++++++++++++++++++++----------------------- tests/ovn-northd.at | 2 +- 3 files changed, 143 insertions(+), 184 deletions(-) diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 75484c5cd..ae799cda9 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -3688,9 +3688,6 @@ build_ports(struct northd_context *ctx, sset_destroy(&active_ha_chassis_grps); } -/* XXX: The 'ovn_lflow_add_unique*()' functions should be used for logical - * flows using a multicast group. - * See the comment on 'ovn_lflow_add_unique()' for details. */ struct multicast_group { const char *name; uint16_t key; /* OVN_MIN_MULTICAST...OVN_MAX_MULTICAST. */ @@ -4107,7 +4104,7 @@ static struct hashrow_locks lflow_locks; * Version to use when locking is required. */ static void -do_ovn_lflow_add(struct hmap *lflow_map, bool shared, struct ovn_datapath *od, +do_ovn_lflow_add(struct hmap *lflow_map, struct ovn_datapath *od, uint32_t hash, enum ovn_stage stage, uint16_t priority, const char *match, const char *actions, const struct ovsdb_idl_row *stage_hint, @@ -4117,7 +4114,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, bool shared, struct ovn_datapath *od, struct ovn_lflow *old_lflow; struct ovn_lflow *lflow; - if (shared && use_logical_dp_groups) { + if (use_logical_dp_groups) { old_lflow = ovn_lflow_find(lflow_map, NULL, stage, priority, match, actions, hash); if (old_lflow) { @@ -4141,7 +4138,7 @@ do_ovn_lflow_add(struct hmap *lflow_map, bool shared, struct ovn_datapath *od, static void ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, enum ovn_stage stage, uint16_t priority, - const char *match, const char *actions, bool shared, + const char *match, const char *actions, const struct ovsdb_idl_row *stage_hint, const char *where) { ovs_assert(ovn_stage_to_datapath_type(stage) == ovn_datapath_get_type(od)); @@ -4155,11 +4152,11 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, if (use_logical_dp_groups && use_parallel_build) { lock_hash_row(&lflow_locks, hash); - do_ovn_lflow_add(lflow_map, shared, od, hash, stage, priority, match, + do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, actions, stage_hint, where); unlock_hash_row(&lflow_locks, hash); } else { - do_ovn_lflow_add(lflow_map, shared, od, hash, stage, priority, match, + do_ovn_lflow_add(lflow_map, od, hash, stage, priority, match, actions, stage_hint, where); } } @@ -4167,30 +4164,11 @@ ovn_lflow_add_at(struct hmap *lflow_map, struct ovn_datapath *od, /* Adds a row with the specified contents to the Logical_Flow table. */ #define ovn_lflow_add_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \ ACTIONS, STAGE_HINT) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \ + ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ STAGE_HINT, OVS_SOURCE_LOCATOR) #define ovn_lflow_add(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, true, \ - NULL, OVS_SOURCE_LOCATOR) - -/* Adds a row with the specified contents to the Logical_Flow table. - * Combining of this logical flow with already existing ones, e.g., by using - * Logical Datapath Groups, is forbidden. - * - * XXX: ovn-controller assumes that a logical flow using multicast group always - * comes after or in the same database update with the corresponding - * multicast group. That will not be the case with datapath groups. - * For this reason, the 'ovn_lflow_add_unique*()' functions should be used - * for such logical flows. - */ -#define ovn_lflow_add_unique_with_hint(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, \ - ACTIONS, STAGE_HINT) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \ - STAGE_HINT, OVS_SOURCE_LOCATOR) - -#define ovn_lflow_add_unique(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS) \ - ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, false, \ + ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \ NULL, OVS_SOURCE_LOCATOR) static struct ovn_lflow * @@ -6461,9 +6439,8 @@ build_lswitch_rport_arp_req_self_orig_flow(struct ovn_port *op, ds_put_format(&match, "eth.src == %s && (arp.op == 1 || nd_ns)", ds_cstr(ð_src)); - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, priority, - ds_cstr(&match), - "outport = \""MC_FLOOD_L2"\"; output;"); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, priority, ds_cstr(&match), + "outport = \""MC_FLOOD_L2"\"; output;"); sset_destroy(&all_eth_addrs); ds_destroy(ð_src); @@ -6510,9 +6487,9 @@ build_lswitch_rport_arp_req_flow_for_ip(struct ds *ip_match, ds_put_format(&actions, "clone {outport = %s; output; }; " "outport = \""MC_FLOOD_L2"\"; output;", patch_op->json_key); - ovn_lflow_add_unique_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, - priority, ds_cstr(&match), - ds_cstr(&actions), stage_hint); + ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, + priority, ds_cstr(&match), + ds_cstr(&actions), stage_hint); } else { ds_put_format(&actions, "outport = %s; output;", patch_op->json_key); ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP, priority, @@ -6866,9 +6843,9 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *lflows) "outport = get_fdb(eth.dst); next;"); if (od->has_unknown) { - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50, - "outport == \"none\"", - "outport = \""MC_UNKNOWN "\"; output;"); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50, + "outport == \"none\"", + "outport = \""MC_UNKNOWN "\"; output;"); } else { ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50, "outport == \"none\"", "drop;"); @@ -7312,26 +7289,26 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, } ds_put_cstr(actions, "igmp;"); /* Punt IGMP traffic to controller. */ - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100, - "ip4 && ip.proto == 2", ds_cstr(actions)); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 100, + "ip4 && ip.proto == 2", ds_cstr(actions)); /* Punt MLD traffic to controller. */ - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 100, - "mldv1 || mldv2", ds_cstr(actions)); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 100, + "mldv1 || mldv2", ds_cstr(actions)); /* Flood all IP multicast traffic destined to 224.0.0.X to all * ports - RFC 4541, section 2.1.2, item 2. */ - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 85, - "ip4.mcast && ip4.dst == 224.0.0.0/24", - "outport = \""MC_FLOOD"\"; output;"); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 85, + "ip4.mcast && ip4.dst == 224.0.0.0/24", + "outport = \""MC_FLOOD"\"; output;"); /* Flood all IPv6 multicast traffic destined to reserved * multicast IPs (RFC 4291, 2.7.1). */ - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 85, - "ip6.mcast_flood", - "outport = \""MC_FLOOD"\"; output;"); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 85, + "ip6.mcast_flood", + "outport = \""MC_FLOOD"\"; output;"); /* Forward uregistered IP multicast to routers with relay enabled * and to any ports configured to flood IP multicast traffic. @@ -7361,14 +7338,14 @@ build_lswitch_destination_lookup_bmcast(struct ovn_datapath *od, ds_put_cstr(actions, "drop;"); } - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 80, - "ip4.mcast || ip6.mcast", - ds_cstr(actions)); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 80, + "ip4.mcast || ip6.mcast", + ds_cstr(actions)); } } - ovn_lflow_add_unique(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast", - "outport = \""MC_FLOOD"\"; output;"); + ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast", + "outport = \""MC_FLOOD"\"; output;"); } } @@ -7446,8 +7423,8 @@ build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group, ds_put_format(actions, "outport = \"%s\"; output; ", igmp_group->mcgroup.name); - ovn_lflow_add_unique(lflows, igmp_group->datapath, S_SWITCH_IN_L2_LKUP, - 90, ds_cstr(match), ds_cstr(actions)); + ovn_lflow_add(lflows, igmp_group->datapath, S_SWITCH_IN_L2_LKUP, + 90, ds_cstr(match), ds_cstr(actions)); } } @@ -9988,15 +9965,15 @@ build_mcast_lookup_flows_for_lrouter( } ds_put_format(actions, "outport = \"%s\"; ip.ttl--; next;", igmp_group->mcgroup.name); - ovn_lflow_add_unique(lflows, od, S_ROUTER_IN_IP_ROUTING, 500, - ds_cstr(match), ds_cstr(actions)); + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 500, + ds_cstr(match), ds_cstr(actions)); } /* If needed, flood unregistered multicast on statically configured * ports. Otherwise drop any multicast traffic. */ if (od->mcast_info.rtr.flood_static) { - ovn_lflow_add_unique(lflows, od, S_ROUTER_IN_IP_ROUTING, 450, + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 450, "ip4.mcast || ip6.mcast", "clone { " "outport = \""MC_STATIC"\"; " diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 3afa80a3b..3b2f74cdf 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -1604,12 +1604,6 @@ function mFF_N_LOG_REGS() : bit<32> = 10 * * - There's a setting "use_logical_dp_groups" that globally * enables or disables this feature. - * - * - Some flows can't use this feature even if it's globally - * enabled, due to ovn-controller bugs (see commit bfed224006750 - * "northd: Add support for Logical Datapath Groups."). Flows - * that can't be shared must get added into AnnotatedFlow with - * 'shared' set to 'false', instead of Flow. */ relation Flow( @@ -1631,12 +1625,6 @@ UseLogicalDatapathGroups[false] :- Unit(), not nb in nb::NB_Global(). -relation AnnotatedFlow(f: Flow, shared: bool) -AnnotatedFlow(f, b) :- UseLogicalDatapathGroups[b], Flow[f]. - -relation UniqueFlow[Flow] -AnnotatedFlow(f, false) :- UniqueFlow[f]. - relation AggregatedFlow ( logical_datapaths: Set, stage: Stage, @@ -1651,15 +1639,17 @@ AggregatedFlow(.logical_datapaths = g.to_set(), .__match = __match, .actions = actions, .external_ids = external_ids) :- - AnnotatedFlow(Flow{logical_datapath, stage, priority, __match, actions, external_ids}, true), - var g = logical_datapath.group_by((stage, priority, __match, actions, external_ids)). + Flow(logical_datapath, stage, priority, __match, actions, external_ids), + var g = logical_datapath.group_by((stage, priority, __match, actions, external_ids)), + UseLogicalDatapathGroups[true]. AggregatedFlow(.logical_datapaths = set_singleton(logical_datapath), .stage = stage, .priority = priority, .__match = __match, .actions = actions, .external_ids = external_ids) :- - AnnotatedFlow(Flow{logical_datapath, stage, priority, __match, actions, external_ids}, false). + Flow(logical_datapath, stage, priority, __match, actions, external_ids), + UseLogicalDatapathGroups[false]. for (f in AggregatedFlow()) { var pipeline = if (f.stage.pipeline == Ingress) "ingress" else "egress" in @@ -3813,42 +3803,42 @@ for (sw in &Switch(._uuid = ls_uuid, .mcast_cfg = mcast_cfg) } } in { /* Punt IGMP traffic to controller. */ - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 100, - .__match = "ip4 && ip.proto == 2", - .actions = "${igmp_act}", - .external_ids = map_empty()}]; + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 100, + .__match = "ip4 && ip.proto == 2", + .actions = "${igmp_act}", + .external_ids = map_empty()); /* Punt MLD traffic to controller. */ - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 100, - .__match = "mldv1 || mldv2", - .actions = "${igmp_act}", - .external_ids = map_empty()}]; + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 100, + .__match = "mldv1 || mldv2", + .actions = "${igmp_act}", + .external_ids = map_empty()); /* Flood all IP multicast traffic destined to 224.0.0.X to * all ports - RFC 4541, section 2.1.2, item 2. */ var flood = json_string_escape(mC_FLOOD().0) in - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 85, - .__match = "ip4.mcast && ip4.dst == 224.0.0.0/24", - .actions = "outport = ${flood}; output;", - .external_ids = map_empty()}]; + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 85, + .__match = "ip4.mcast && ip4.dst == 224.0.0.0/24", + .actions = "outport = ${flood}; output;", + .external_ids = map_empty()); /* Flood all IPv6 multicast traffic destined to reserved * multicast IPs (RFC 4291, 2.7.1). */ var flood = json_string_escape(mC_FLOOD().0) in - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 85, - .__match = "ip6.mcast_flood", - .actions = "outport = ${flood}; output;", - .external_ids = map_empty()}]; + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 85, + .__match = "ip6.mcast_flood", + .actions = "outport = ${flood}; output;", + .external_ids = map_empty()); /* Forward uregistered IP multicast to routers with relay * enabled and to any ports configured to flood IP @@ -3882,13 +3872,13 @@ for (sw in &Switch(._uuid = ls_uuid, .mcast_cfg = mcast_cfg) "" } } in - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 80, - .__match = "ip4.mcast || ip6.mcast", - .actions = - "${relay_act}${static_act}${drop_act}", - .external_ids = map_empty()}] + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 80, + .__match = "ip4.mcast || ip6.mcast", + .actions = + "${relay_act}${static_act}${drop_act}", + .external_ids = map_empty()) } } } @@ -3936,14 +3926,14 @@ for (IgmpSwitchMulticastGroup(.address = address, .switch = sw)) { "" } } in - UniqueFlow[Flow{.logical_datapath = sw._uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 90, - .__match = "eth.mcast && ${ipX} && ${ipX}.dst == ${address}", - .actions = - "${relay_act} ${static_act} outport = \"${address}\"; " - "output;", - .external_ids = map_empty()}] + Flow(.logical_datapath = sw._uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 90, + .__match = "eth.mcast && ${ipX} && ${ipX}.dst == ${address}", + .actions = + "${relay_act} ${static_act} outport = \"${address}\"; " + "output;", + .external_ids = map_empty()) } } } @@ -4010,12 +4000,12 @@ Flow(.logical_datapath = sp.sw._uuid, * (priority 100). */ for (ls in nb::Logical_Switch) { var mc_flood = json_string_escape(mC_FLOOD().0) in - UniqueFlow[Flow{.logical_datapath = ls._uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 70, - .__match = "eth.mcast", - .actions = "outport = ${mc_flood}; output;", - .external_ids = map_empty()}] + Flow(.logical_datapath = ls._uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 70, + .__match = "eth.mcast", + .actions = "outport = ${mc_flood}; output;", + .external_ids = map_empty()) } /* Ingress table L2_LKUP: Destination lookup, unicast handling (priority 50). @@ -4064,12 +4054,12 @@ function lrouter_port_ip_reachable(rp: Intern, addr: v46_ip): bool { }; false } -UniqueFlow[Flow{.logical_datapath = sw._uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 75, - .__match = __match, - .actions = actions, - .external_ids = stage_hint(sp.lsp._uuid)}] :- +Flow(.logical_datapath = sw._uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 75, + .__match = __match, + .actions = actions, + .external_ids = stage_hint(sp.lsp._uuid)) :- sp in &SwitchPort(.sw = sw@&Switch{.has_non_router_port = true}, .peer = Some{rp}), rp.is_enabled(), var eth_src_set = { @@ -4152,39 +4142,37 @@ function get_arp_forward_ips(rp: Intern): (Set, Set) * delivers to patch ports) but we're bypassing multicast_groups. * (This is why we match against fLAGBIT_NOT_VXLAN() here.) */ -AnnotatedFlow(.f = Flow{.logical_datapath = sw._uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 80, - .__match = fLAGBIT_NOT_VXLAN() ++ - " && arp.op == 1 && arp.tpa == { " ++ - all_ips_v4.to_vec().join(", ") ++ "}", - .actions = if (sw.has_non_router_port) { - "clone {outport = ${sp.json_name}; output; }; " - "outport = ${mc_flood_l2}; output;" - } else { - "outport = ${sp.json_name}; output;" - }, - .external_ids = stage_hint(sp.lsp._uuid)}, - .shared = not sw.has_non_router_port) :- +Flow(.logical_datapath = sw._uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 80, + .__match = fLAGBIT_NOT_VXLAN() ++ + " && arp.op == 1 && arp.tpa == { " ++ + all_ips_v4.to_vec().join(", ") ++ "}", + .actions = if (sw.has_non_router_port) { + "clone {outport = ${sp.json_name}; output; }; " + "outport = ${mc_flood_l2}; output;" + } else { + "outport = ${sp.json_name}; output;" + }, + .external_ids = stage_hint(sp.lsp._uuid)) :- sp in &SwitchPort(.sw = sw, .peer = Some{rp}), rp.is_enabled(), (var all_ips_v4, _) = get_arp_forward_ips(rp), not all_ips_v4.is_empty(), var mc_flood_l2 = json_string_escape(mC_FLOOD_L2().0). -AnnotatedFlow(.f = Flow{.logical_datapath = sw._uuid, - .stage = s_SWITCH_IN_L2_LKUP(), - .priority = 80, - .__match = fLAGBIT_NOT_VXLAN() ++ - " && nd_ns && nd.target == { " ++ - all_ips_v6.to_vec().join(", ") ++ "}", - .actions = if (sw.has_non_router_port) { - "clone {outport = ${sp.json_name}; output; }; " - "outport = ${mc_flood_l2}; output;" - } else { - "outport = ${sp.json_name}; output;" - }, - .external_ids = stage_hint(sp.lsp._uuid)}, - .shared = not sw.has_non_router_port) :- +Flow(.logical_datapath = sw._uuid, + .stage = s_SWITCH_IN_L2_LKUP(), + .priority = 80, + .__match = fLAGBIT_NOT_VXLAN() ++ + " && nd_ns && nd.target == { " ++ + all_ips_v6.to_vec().join(", ") ++ "}", + .actions = if (sw.has_non_router_port) { + "clone {outport = ${sp.json_name}; output; }; " + "outport = ${mc_flood_l2}; output;" + } else { + "outport = ${sp.json_name}; output;" + }, + .external_ids = stage_hint(sp.lsp._uuid)) :- sp in &SwitchPort(.sw = sw, .peer = Some{rp}), rp.is_enabled(), (_, var all_ips_v6) = get_arp_forward_ips(rp), @@ -4280,22 +4268,17 @@ for (sw in &Switch(._uuid = ls_uuid)) { .actions = "outport = get_fdb(eth.dst); next;", .external_ids = map_empty()); - if (sw.has_unknown_ports) { - var mc_unknown = json_string_escape(mC_UNKNOWN().0) in - UniqueFlow[Flow{.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_UNKNOWN(), - .priority = 50, - .__match = "outport == \"none\"", - .actions = "outport = ${mc_unknown}; output;", - .external_ids = map_empty()}] - } else { - Flow(.logical_datapath = ls_uuid, - .stage = s_SWITCH_IN_L2_UNKNOWN(), - .priority = 50, - .__match = "outport == \"none\"", - .actions = "drop;", - .external_ids = map_empty()) - }; + Flow(.logical_datapath = ls_uuid, + .stage = s_SWITCH_IN_L2_UNKNOWN(), + .priority = 50, + .__match = "outport == \"none\"", + .actions = if (sw.has_unknown_ports) { + var mc_unknown = json_string_escape(mC_UNKNOWN().0); + "outport = ${mc_unknown}; output;" + } else { + "drop;" + }, + .external_ids = map_empty()); Flow(.logical_datapath = ls_uuid, .stage = s_SWITCH_IN_L2_UNKNOWN(), @@ -6639,14 +6622,14 @@ for (IgmpRouterMulticastGroup(address, rtr, ports)) { } in Some{var ip} = ip46_parse(address) in var ipX = ip.ipX() in - UniqueFlow[Flow{.logical_datapath = rtr._uuid, - .stage = s_ROUTER_IN_IP_ROUTING(), - .priority = 500, - .__match = "${ipX} && ${ipX}.dst == ${address} ", - .actions = - "${static_act}outport = ${json_string_escape(address)}; " - "ip.ttl--; next;", - .external_ids = map_empty()}] + Flow(.logical_datapath = rtr._uuid, + .stage = s_ROUTER_IN_IP_ROUTING(), + .priority = 500, + .__match = "${ipX} && ${ipX}.dst == ${address} ", + .actions = + "${static_act}outport = ${json_string_escape(address)}; " + "ip.ttl--; next;", + .external_ids = map_empty()) } } @@ -6665,13 +6648,12 @@ for (RouterMcastFloodPorts(rtr, flood_ports) if rtr.mcast_cfg.relay) { } else { "drop;" } in - AnnotatedFlow(.f = Flow{.logical_datapath = rtr._uuid, - .stage = s_ROUTER_IN_IP_ROUTING(), - .priority = 450, - .__match = "ip4.mcast || ip6.mcast", - .actions = actions, - .external_ids = map_empty()}, - .shared = not flood_static) + Flow(.logical_datapath = rtr._uuid, + .stage = s_ROUTER_IN_IP_ROUTING(), + .priority = 450, + .__match = "ip4.mcast || ip6.mcast", + .actions = actions, + .external_ids = map_empty()) } /* Logical router ingress table POLICY: Policy. diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index d81975cb1..130887b18 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2487,7 +2487,7 @@ check_row_count Logical_DP_Group 0 dnl Number of logical flows that depends on logical switch or multicast group. dnl These will not be combined. -n_flows_specific=$(ovn-sbctl --bare find Logical_Flow | grep -cE 'swp|_MC_') +n_flows_specific=$(ovn-sbctl --bare find Logical_Flow | grep -cE 'swp') echo "Number of specific flows: "${n_flows_specific} dnl Both logical switches configured identically, so there should be same From patchwork Mon Jun 21 06:51:34 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1494916 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=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 4G7gFJ3YgNz9s5R for ; Mon, 21 Jun 2021 16:52:18 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id E191A4034A; Mon, 21 Jun 2021 06:52:14 +0000 (UTC) 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 wm3fqitWRru0; Mon, 21 Jun 2021 06:52:13 +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 AF29040381; Mon, 21 Jun 2021 06:52:06 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 5839DC000E; Mon, 21 Jun 2021 06:52:06 +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 05B24C000C for ; Mon, 21 Jun 2021 06:52:05 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id D26BC40338 for ; Mon, 21 Jun 2021 06:52:04 +0000 (UTC) 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 TuqrpddiBq8n for ; Mon, 21 Jun 2021 06:52:04 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp4.osuosl.org (Postfix) with ESMTPS id EB21A40337 for ; Mon, 21 Jun 2021 06:52:03 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id D1A66E0007; Mon, 21 Jun 2021 06:52:00 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Sun, 20 Jun 2021 23:51:34 -0700 Message-Id: <20210621065135.1236639-3-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621065135.1236639-1-hzhou@ovn.org> References: <20210621065135.1236639-1-hzhou@ovn.org> MIME-Version: 1.0 Cc: Dumitru Ceara Subject: [ovs-dev] [PATCH ovn v3 2/3] ovn-controller: Always monitor all logical datapath groups. 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" Always monitor all logical datapath groups. Otherwise, DPG updates may be received *after* the lflows using it are seen by ovn-controller. Since the number of DPGs are relatively small, we monitor all DPGs to avoid the unnecessarily extra control plane round trip for the lflows to be processed. Acked-by: Dumitru Ceara Signed-off-by: Han Zhou --- controller/ovn-controller.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 1cfe4b713..c28fd6f2d 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -192,10 +192,15 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, struct ovsdb_idl_condition igmp = OVSDB_IDL_CONDITION_INIT(&igmp); struct ovsdb_idl_condition chprv = OVSDB_IDL_CONDITION_INIT(&chprv); + /* Always monitor all logical datapath groups. Otherwise, DPG updates may + * be received *after* the lflows using it are seen by ovn-controller. + * Since the number of DPGs are relatively small, we monitor all DPGs to + * avoid the unnecessarily extra wake-ups of ovn-controller. */ + ovsdb_idl_condition_add_clause_true(&ldpg); + if (monitor_all) { ovsdb_idl_condition_add_clause_true(&pb); ovsdb_idl_condition_add_clause_true(&lf); - ovsdb_idl_condition_add_clause_true(&ldpg); ovsdb_idl_condition_add_clause_true(&mb); ovsdb_idl_condition_add_clause_true(&mg); ovsdb_idl_condition_add_clause_true(&dns); @@ -259,8 +264,6 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, sbrec_port_binding_add_clause_datapath(&pb, OVSDB_F_EQ, uuid); sbrec_logical_flow_add_clause_logical_datapath(&lf, OVSDB_F_EQ, uuid); - sbrec_logical_dp_group_add_clause_datapaths( - &ldpg, OVSDB_F_INCLUDES, &uuid, 1); sbrec_mac_binding_add_clause_datapath(&mb, OVSDB_F_EQ, uuid); sbrec_multicast_group_add_clause_datapath(&mg, OVSDB_F_EQ, uuid); sbrec_dns_add_clause_datapaths(&dns, OVSDB_F_INCLUDES, &uuid, 1); From patchwork Mon Jun 21 06:51:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Han Zhou X-Patchwork-Id: 1494919 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=140.211.166.137; helo=smtp4.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp4.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (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 4G7gFV5Fsvz9sW7 for ; Mon, 21 Jun 2021 16:52:30 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 79E4940421; Mon, 21 Jun 2021 06:52:25 +0000 (UTC) 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 TtRXGECSNvoB; Mon, 21 Jun 2021 06:52:22 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp4.osuosl.org (Postfix) with ESMTPS id 2C181403C3; Mon, 21 Jun 2021 06:52:12 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id DE99DC0024; Mon, 21 Jun 2021 06:52:10 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 39CB4C000C for ; Mon, 21 Jun 2021 06:52:07 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 0E8D4606A7 for ; Mon, 21 Jun 2021 06:52:07 +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 LxOlNlDdhjV7 for ; Mon, 21 Jun 2021 06:52:05 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net [217.70.183.196]) by smtp3.osuosl.org (Postfix) with ESMTPS id 7D5CC60645 for ; Mon, 21 Jun 2021 06:52:05 +0000 (UTC) Received: (Authenticated sender: hzhou@ovn.org) by relay4-d.mail.gandi.net (Postfix) with ESMTPSA id AC976E0004; Mon, 21 Jun 2021 06:52:02 +0000 (UTC) From: Han Zhou To: dev@openvswitch.org Date: Sun, 20 Jun 2021 23:51:35 -0700 Message-Id: <20210621065135.1236639-4-hzhou@ovn.org> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210621065135.1236639-1-hzhou@ovn.org> References: <20210621065135.1236639-1-hzhou@ovn.org> MIME-Version: 1.0 Subject: [ovs-dev] [PATCH ovn v3 3/3] ovn-controller: Fix incremental processing for logical port references. 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" If a lflow has an lport name in the match, but when the lflow is processed the port-binding is not seen by ovn-controller, the corresponding openflow will not be created. Later if the port-binding is created/monitored by ovn-controller, the lflow is not reprocessed because the lflow didn't change and ovn-controller doesn't know that the port-binding affects the lflow. This patch fixes the problem by tracking the references when parsing the lflow, even if the port-binding is not found when the lflow is firstly parsed. A test case is also added to cover the scenario. Signed-off-by: Han Zhou --- controller/lflow.c | 63 ++++++++++++++++++++++++++----------- controller/lflow.h | 3 ++ controller/ovn-controller.c | 35 ++++++++++++++++----- include/ovn/expr.h | 2 +- lib/expr.c | 14 +++------ tests/ovn.at | 47 +++++++++++++++++++++++++++ tests/test-ovn.c | 4 +-- utilities/ovn-trace.c | 2 +- 8 files changed, 132 insertions(+), 38 deletions(-) diff --git a/controller/lflow.c b/controller/lflow.c index 34eca135a..b7699a309 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -61,6 +61,7 @@ struct lookup_port_aux { struct condition_aux { struct ovsdb_idl_index *sbrec_port_binding_by_name; + const struct sbrec_datapath_binding *dp; const struct sbrec_chassis *chassis; const struct sset *active_tunnels; const struct sbrec_logical_flow *lflow; @@ -98,6 +99,12 @@ lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp) const struct lookup_port_aux *aux = aux_; + /* Store the name that used to lookup the lport to lflow reference, so that + * in the future when the lport's port binding changes, the logical flow + * that references this lport can be reprocessed. */ + lflow_resource_add(aux->lfrr, REF_TYPE_PORTBINDING, port_name, + &aux->lflow->header_.uuid); + const struct sbrec_port_binding *pb = lport_lookup_by_name(aux->sbrec_port_binding_by_name, port_name); if (pb && pb->datapath == aux->dp) { @@ -149,19 +156,18 @@ is_chassis_resident_cb(const void *c_aux_, const char *port_name) { const struct condition_aux *c_aux = c_aux_; + /* Store the port name that used to lookup the lport to lflow reference, so + * that in the future when the lport's port-binding changes the logical + * flow that references this lport can be reprocessed. */ + lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, port_name, + &c_aux->lflow->header_.uuid); + const struct sbrec_port_binding *pb = lport_lookup_by_name(c_aux->sbrec_port_binding_by_name, port_name); if (!pb) { return false; } - /* Store the port_name to lflow reference. */ - int64_t dp_id = pb->datapath->tunnel_key; - char buf[16]; - get_unique_lport_key(dp_id, pb->tunnel_key, buf, sizeof(buf)); - lflow_resource_add(c_aux->lfrr, REF_TYPE_PORTBINDING, buf, - &c_aux->lflow->header_.uuid); - if (strcmp(pb->type, "chassisredirect")) { /* for non-chassisredirect ports */ return pb->chassis && pb->chassis == c_aux->chassis; @@ -623,8 +629,6 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, int64_t dp_id = dp->tunnel_key; char buf[16]; get_unique_lport_key(dp_id, port_id, buf, sizeof(buf)); - lflow_resource_add(l_ctx_out->lfrr, REF_TYPE_PORTBINDING, buf, - &lflow->header_.uuid); if (!sset_contains(l_ctx_in->local_lport_ids, buf)) { VLOG_DBG("lflow "UUID_FMT " port %s in match is not local, skip", @@ -788,6 +792,7 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, }; struct condition_aux cond_aux = { .sbrec_port_binding_by_name = l_ctx_in->sbrec_port_binding_by_name, + .dp = dp, .chassis = l_ctx_in->chassis, .active_tunnels = l_ctx_in->active_tunnels, .lflow = lflow, @@ -805,7 +810,6 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, struct hmap *matches = NULL; size_t matches_size = 0; - bool is_cr_cond_present = false; bool pg_addr_set_ref = false; uint32_t n_conjs = 0; @@ -843,8 +847,8 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, case LCACHE_T_NONE: case LCACHE_T_CONJ_ID: case LCACHE_T_EXPR: - expr = expr_evaluate_condition(expr, is_chassis_resident_cb, &cond_aux, - &is_cr_cond_present); + expr = expr_evaluate_condition(expr, is_chassis_resident_cb, + &cond_aux); expr = expr_normalize(expr); break; case LCACHE_T_MATCHES: @@ -883,7 +887,9 @@ consider_logical_flow__(const struct sbrec_logical_flow *lflow, /* Cache new entry if caching is enabled. */ if (lflow_cache_is_enabled(l_ctx_out->lflow_cache)) { - if (cached_expr && !is_cr_cond_present) { + if (cached_expr + && !lflow_ref_lookup(&l_ctx_out->lfrr->lflow_ref_table, + &lflow->header_.uuid)) { lflow_cache_add_matches(l_ctx_out->lflow_cache, &lflow->header_.uuid, matches, matches_size); @@ -1746,21 +1752,42 @@ lflow_processing_end: return handled; } +/* Handles a port-binding change that is possibly related to a lport's + * residence status on this chassis. */ bool lflow_handle_flows_for_lport(const struct sbrec_port_binding *pb, struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) { bool changed; - int64_t dp_id = pb->datapath->tunnel_key; - char pb_ref_name[16]; - get_unique_lport_key(dp_id, pb->tunnel_key, pb_ref_name, - sizeof(pb_ref_name)); - return lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb_ref_name, + return lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb->logical_port, l_ctx_in, l_ctx_out, &changed); } +/* Handles port-binding add/deletions. */ +bool +lflow_handle_changed_port_bindings(struct lflow_ctx_in *l_ctx_in, + struct lflow_ctx_out *l_ctx_out) +{ + bool ret = true; + bool changed; + const struct sbrec_port_binding *pb; + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, + l_ctx_in->port_binding_table) { + if (!sbrec_port_binding_is_new(pb) + && !sbrec_port_binding_is_deleted(pb)) { + continue; + } + if (!lflow_handle_changed_ref(REF_TYPE_PORTBINDING, pb->logical_port, + l_ctx_in, l_ctx_out, &changed)) { + ret = false; + break; + } + } + return ret; +} + bool lflow_handle_changed_mc_groups(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out) diff --git a/controller/lflow.h b/controller/lflow.h index e98edf81d..9d8882ae5 100644 --- a/controller/lflow.h +++ b/controller/lflow.h @@ -130,6 +130,7 @@ struct lflow_ctx_in { struct ovsdb_idl_index *sbrec_logical_flow_by_logical_datapath; struct ovsdb_idl_index *sbrec_logical_flow_by_logical_dp_group; struct ovsdb_idl_index *sbrec_port_binding_by_name; + const struct sbrec_port_binding_table *port_binding_table; const struct sbrec_dhcp_options_table *dhcp_options_table; const struct sbrec_dhcpv6_options_table *dhcpv6_options_table; const struct sbrec_datapath_binding_table *dp_binding_table; @@ -182,4 +183,6 @@ bool lflow_handle_flows_for_lport(const struct sbrec_port_binding *, struct lflow_ctx_out *); bool lflow_handle_changed_mc_groups(struct lflow_ctx_in *, struct lflow_ctx_out *); +bool lflow_handle_changed_port_bindings(struct lflow_ctx_in *, + struct lflow_ctx_out *); #endif /* controller/lflow.h */ diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index c28fd6f2d..8136afe5c 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1966,6 +1966,10 @@ init_lflow_ctx(struct engine_node *node, engine_get_input("SB_multicast_group", node), "name_datapath"); + struct sbrec_port_binding_table *port_binding_table = + (struct sbrec_port_binding_table *)EN_OVSDB_GET( + engine_get_input("SB_port_binding", node)); + struct sbrec_dhcp_options_table *dhcp_table = (struct sbrec_dhcp_options_table *)EN_OVSDB_GET( engine_get_input("SB_dhcp_options", node)); @@ -2029,6 +2033,7 @@ init_lflow_ctx(struct engine_node *node, l_ctx_in->sbrec_logical_flow_by_logical_dp_group = sbrec_logical_flow_by_dp_group; l_ctx_in->sbrec_port_binding_by_name = sbrec_port_binding_by_name; + l_ctx_in->port_binding_table = port_binding_table; l_ctx_in->dhcp_options_table = dhcp_table; l_ctx_in->dhcpv6_options_table = dhcpv6_table; l_ctx_in->mac_binding_table = mac_binding_table; @@ -2221,6 +2226,25 @@ lflow_output_sb_multicast_group_handler(struct engine_node *node, void *data) return true; } +static bool +lflow_output_sb_port_binding_handler(struct engine_node *node, void *data) +{ + struct ed_type_runtime_data *rt_data = + engine_get_input_data("runtime_data", node); + + struct ed_type_lflow_output *lfo = data; + + struct lflow_ctx_in l_ctx_in; + struct lflow_ctx_out l_ctx_out; + init_lflow_ctx(node, rt_data, lfo, &l_ctx_in, &l_ctx_out); + if (!lflow_handle_changed_port_bindings(&l_ctx_in, &l_ctx_out)) { + return false; + } + + engine_set_node_state(node, EN_UPDATED); + return true; +} + static bool _lflow_output_resource_ref_handler(struct engine_node *node, void *data, enum ref_type ref_type) @@ -2285,8 +2309,9 @@ _lflow_output_resource_ref_handler(struct engine_node *node, void *data, break; case REF_TYPE_PORTBINDING: - /* This ref type is handled in the - * flow_output_runtime_data_handler. */ + /* This ref type is handled in: + * - flow_output_runtime_data_handler + * - flow_output_sb_port_binding_handler. */ case REF_TYPE_MC_GROUP: /* This ref type is handled in the * flow_output_sb_multicast_group_handler. */ @@ -2895,12 +2920,8 @@ main(int argc, char *argv[]) engine_add_input(&en_lflow_output, &en_sb_chassis, pflow_lflow_output_sb_chassis_handler); - /* Any changes to the port binding, need not be handled - * for lflow_outout engine. We still need sb_port_binding - * as input to access the port binding data in lflow.c and - * hence the noop handler. */ engine_add_input(&en_lflow_output, &en_sb_port_binding, - engine_noop_handler); + lflow_output_sb_port_binding_handler); engine_add_input(&en_lflow_output, &en_ovs_open_vswitch, NULL); engine_add_input(&en_lflow_output, &en_ovs_bridge, NULL); diff --git a/include/ovn/expr.h b/include/ovn/expr.h index de90e87e1..3b5653f7b 100644 --- a/include/ovn/expr.h +++ b/include/ovn/expr.h @@ -438,7 +438,7 @@ struct expr *expr_evaluate_condition( struct expr *, bool (*is_chassis_resident)(const void *c_aux, const char *port_name), - const void *c_aux, bool *condition_present); + const void *c_aux); struct expr *expr_normalize(struct expr *); bool expr_honors_invariants(const struct expr *); diff --git a/lib/expr.c b/lib/expr.c index f728f9537..e3f6bb892 100644 --- a/lib/expr.c +++ b/lib/expr.c @@ -2121,16 +2121,13 @@ static struct expr * expr_evaluate_condition__(struct expr *expr, bool (*is_chassis_resident)(const void *c_aux, const char *port_name), - const void *c_aux, bool *condition_present) + const void *c_aux) { bool result; switch (expr->cond.type) { case EXPR_COND_CHASSIS_RESIDENT: result = is_chassis_resident(c_aux, expr->cond.string); - if (condition_present != NULL) { - *condition_present = true; - } break; default: @@ -2146,7 +2143,7 @@ struct expr * expr_evaluate_condition(struct expr *expr, bool (*is_chassis_resident)(const void *c_aux, const char *port_name), - const void *c_aux, bool *condition_present) + const void *c_aux) { struct expr *sub, *next; @@ -2156,15 +2153,14 @@ expr_evaluate_condition(struct expr *expr, LIST_FOR_EACH_SAFE (sub, next, node, &expr->andor) { ovs_list_remove(&sub->node); struct expr *e = expr_evaluate_condition(sub, is_chassis_resident, - c_aux, condition_present); + c_aux); e = expr_fix(e); expr_insert_andor(expr, next, e); } return expr_fix(expr); case EXPR_T_CONDITION: - return expr_evaluate_condition__(expr, is_chassis_resident, c_aux, - condition_present); + return expr_evaluate_condition__(expr, is_chassis_resident, c_aux); case EXPR_T_CMP: case EXPR_T_BOOLEAN: @@ -3517,7 +3513,7 @@ expr_parse_microflow__(struct lexer *lexer, e = expr_simplify(e); e = expr_evaluate_condition(e, microflow_is_chassis_resident_cb, - NULL, NULL); + NULL); e = expr_normalize(e); struct match m = MATCH_CATCHALL_INITIALIZER; diff --git a/tests/ovn.at b/tests/ovn.at index bc494fcad..e6d609b5b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -26782,3 +26782,50 @@ AT_CHECK([ovs-ofctl dump-flows br-int "nw_src=10.0.0.0/24" | \ OVN_CLEANUP([hv1]) AT_CLEANUP ]) + +# Test when a logical port name is used in an ACL before it is created. When it +# is created later, the ACL should be programmed as openflow rules by +# ovn-controller. Although this is not likely to happen in real world use +# cases, it is possible that a port-binding is observed by ovn-controller AFTER +# an lflow that references the port is processed. So this test case is to make +# sure the incremental processing in ovn-controller reprocesses the lflow when +# the port-binding is observed. +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- ACL referencing lport before creation]) +ovn_start + +net_add n1 + +sim_add hv1 +as hv1 +check ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +# Bind both lp1 and lp2 on the chasis. +check ovs-vsctl add-port br-int lp1 -- set interface lp1 external_ids:iface-id=lp1 +check ovs-vsctl add-port br-int lp2 -- set interface lp2 external_ids:iface-id=lp2 + +# Create only lport lp1, but not lp2. +check ovn-nbctl ls-add lsw0 +check ovn-nbctl lsp-add lsw0 lp1 \ + -- lsp-set-addresses lp1 "f0:00:00:00:00:01 10.0.0.11" + +# Each lport is referenced by an ACL. +check ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp1" && ip4.src == 10.0.0.111' allow-related +check ovn-nbctl acl-add lsw0 to-lport 1002 'outport == "lp2" && ip4.src == 10.0.0.222' allow-related + +# The first ACL should be programmed. +check ovn-nbctl --wait=hv sync +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10.0.0.111], [0], [ignore]) + +# Now create the lport lp2. +check ovn-nbctl lsp-add lsw0 lp2 \ + -- lsp-set-addresses lp2 "f0:00:00:00:00:02 10.0.0.22" + +check ovn-nbctl --wait=hv sync +# Now the second ACL should be programmed. +AT_CHECK([ovs-ofctl dump-flows br-int table=44 | grep 10.0.0.222], [0], [ignore]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) diff --git a/tests/test-ovn.c b/tests/test-ovn.c index 98cc2c503..c6d8b287b 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -318,7 +318,7 @@ test_parse_expr__(int steps) if (steps > 1) { expr = expr_simplify(expr); expr = expr_evaluate_condition(expr, is_chassis_resident_cb, - &ports, NULL); + &ports); } if (steps > 2) { expr = expr_normalize(expr); @@ -922,7 +922,7 @@ test_tree_shape_exhaustively(struct expr *expr, struct shash *symtab, modified = expr_simplify(expr_clone(expr)); modified = expr_evaluate_condition( modified, tree_shape_is_chassis_resident_cb, - NULL, NULL); + NULL); ovs_assert(expr_honors_invariants(modified)); if (operation >= OP_NORMALIZE) { diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index 49463c5c2..5d016b757 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -973,7 +973,7 @@ parse_lflow_for_datapath(const struct sbrec_logical_flow *sblf, match = expr_simplify(match); match = expr_evaluate_condition(match, ovntrace_is_chassis_resident, - NULL, NULL); + NULL); } struct ovntrace_flow *flow = xzalloc(sizeof *flow);