[ovs-dev] [PATCH ovn 2/4] ovn-northd: Remove lflow_add_unique.
Han Zhou
hzhou at ovn.org
Fri May 28 19:23:37 UTC 2021
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 <hzhou at ovn.org>
---
northd/ovn-northd.c | 24 ++---
northd/ovn_northd.dl | 220 +++++++++++++++++++++----------------------
tests/ovn-northd.at | 2 +-
3 files changed, 118 insertions(+), 128 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ca56a6efb..89d86596b 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6443,7 +6443,7 @@ 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,
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, priority,
ds_cstr(&match),
"outport = \""MC_FLOOD_L2"\"; output;");
@@ -6498,7 +6498,7 @@ build_lswitch_rport_arp_req_flow_for_ip(struct sset *ips,
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,
+ ovn_lflow_add_with_hint(lflows, od, S_SWITCH_IN_L2_LKUP,
priority, ds_cstr(&match),
ds_cstr(&actions), stage_hint);
} else {
@@ -6854,7 +6854,7 @@ 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,
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
"outport == \"none\"",
"outport = \""MC_UNKNOWN "\"; output;");
} else {
@@ -7300,24 +7300,24 @@ 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,
+ 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,
+ 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,
+ 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,
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 85,
"ip6.mcast_flood",
"outport = \""MC_FLOOD"\"; output;");
@@ -7349,13 +7349,13 @@ 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,
+ 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",
+ ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 70, "eth.mcast",
"outport = \""MC_FLOOD"\"; output;");
}
}
@@ -7434,7 +7434,7 @@ 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,
+ ovn_lflow_add(lflows, igmp_group->datapath, S_SWITCH_IN_L2_LKUP,
90, ds_cstr(match), ds_cstr(actions));
}
}
@@ -9976,7 +9976,7 @@ 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,
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 500,
ds_cstr(match), ds_cstr(actions));
}
@@ -9984,7 +9984,7 @@ build_mcast_lookup_flows_for_lrouter(
* 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 cb8418540..156eee43a 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1605,11 +1605,9 @@ 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.
+ * - It is possible that some flows can't use this feature even if it's
+ * globally enabled. Flows that can't be shared must get added into
+ * AnnotatedFlow with 'shared' set to 'false', instead of Flow.
*/
relation Flow(
@@ -3812,42 +3810,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
@@ -3881,13 +3879,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())
}
}
}
@@ -3935,14 +3933,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())
}
}
}
@@ -4009,12 +4007,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).
@@ -4063,12 +4061,12 @@ function lrouter_port_ip_reachable(rp: Intern<RouterPort>, 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 = {
@@ -4151,39 +4149,37 @@ function get_arp_forward_ips(rp: Intern<RouterPort>): (Set<string>, Set<string>)
* 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),
@@ -4279,22 +4275,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(),
@@ -6638,14 +6629,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())]
}
}
@@ -6664,13 +6655,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 3c2aef4b0..dd20f9e7b 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2495,7 +2495,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
--
2.30.2
More information about the dev
mailing list