[ovs-dev] [PATCH v4 ovn 5/8] northd: get rid of add_router_lb_flow
Lorenzo Bianconi
lorenzo.bianconi at redhat.com
Fri Jul 2 17:16:02 UTC 2021
Remove add_router_lb_flow routine and move leftover lb flow
installation code in build_lrouter_snat_flows_for_lb routine
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
northd/ovn-northd.c | 282 ++++++++++++++++++++------------------------
1 file changed, 128 insertions(+), 154 deletions(-)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 39aa2dd82..d6b10cdb5 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8768,97 +8768,12 @@ enum lb_snat_type {
SKIP_SNAT,
};
-static void
-add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
- enum lb_snat_type snat_type, struct ovn_lb_vip *lb_vip,
- const char *proto, struct nbrec_load_balancer *lb,
- struct sset *nat_entries)
-{
- const char *ip_match = NULL;
- if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) {
- ip_match = "ip4";
- } else {
- ip_match = "ip6";
- }
-
- if (sset_contains(nat_entries, lb_vip->vip_str)) {
- /* The load balancer vip is also present in the NAT entries.
- * So add a high priority lflow to advance the the packet
- * destined to the vip (and the vip port if defined)
- * in the S_ROUTER_IN_UNSNAT stage.
- * There seems to be an issue with ovs-vswitchd. When the new
- * connection packet destined for the lb vip is received,
- * it is dnat'ed in the S_ROUTER_IN_DNAT stage in the dnat
- * conntrack zone. For the next packet, if it goes through
- * unsnat stage, the conntrack flags are not set properly, and
- * it doesn't hit the established state flows in
- * S_ROUTER_IN_DNAT stage. */
- struct ds unsnat_match = DS_EMPTY_INITIALIZER;
- ds_put_format(&unsnat_match, "%s && %s.dst == %s && %s",
- ip_match, ip_match, lb_vip->vip_str, proto);
- if (lb_vip->vip_port) {
- ds_put_format(&unsnat_match, " && %s.dst == %d", proto,
- lb_vip->vip_port);
- }
-
- ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT, 120,
- ds_cstr(&unsnat_match), "next;", &lb->header_);
-
- ds_destroy(&unsnat_match);
- }
-
- if (!od->l3dgw_port || !od->l3redirect_port || !lb_vip->n_backends) {
- return;
- }
-
- /* Add logical flows to UNDNAT the load balanced reverse traffic in
- * the router egress pipleine stage - S_ROUTER_OUT_UNDNAT if the logical
- * router has a gateway router port associated.
- */
- struct ds undnat_match = DS_EMPTY_INITIALIZER;
- ds_put_format(&undnat_match, "%s && (", ip_match);
-
- for (size_t i = 0; i < lb_vip->n_backends; i++) {
- struct ovn_lb_backend *backend = &lb_vip->backends[i];
- ds_put_format(&undnat_match, "(%s.src == %s", ip_match,
- backend->ip_str);
-
- if (backend->port) {
- ds_put_format(&undnat_match, " && %s.src == %d) || ",
- proto, backend->port);
- } else {
- ds_put_cstr(&undnat_match, ") || ");
- }
- }
-
- ds_chomp(&undnat_match, ' ');
- ds_chomp(&undnat_match, '|');
- ds_chomp(&undnat_match, '|');
- ds_chomp(&undnat_match, ' ');
- ds_put_format(&undnat_match, ") && outport == %s && "
- "is_chassis_resident(%s)", od->l3dgw_port->json_key,
- od->l3redirect_port->json_key);
- if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) {
- char *action = xasprintf("flags.%s_snat_for_lb = 1; ct_dnat;",
- snat_type == SKIP_SNAT ? "skip" : "force");
- ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
- ds_cstr(&undnat_match), action,
- &lb->header_);
- free(action);
- } else {
- ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
- ds_cstr(&undnat_match), "ct_dnat;",
- &lb->header_);
- }
-
- ds_destroy(&undnat_match);
-}
-
static void
build_lrouter_nat_lflows_for_lb(struct ovn_lb_vip *lb_vip,
struct ovn_northd_lb *lb,
struct ovn_northd_lb_vip *vips_nb,
- struct hmap *lflows)
+ struct hmap *lflows,
+ struct sset *nat_entries)
{
struct ds action = DS_EMPTY_INITIALIZER;
struct ds match = DS_EMPTY_INITIALIZER;
@@ -8903,10 +8818,72 @@ build_lrouter_nat_lflows_for_lb(struct ovn_lb_vip *lb_vip,
new_match = xasprintf("ct.new && %s", ds_cstr(&match));
est_match = xasprintf("ct.est && %s", ds_cstr(&match));
+ const char *ip_match = NULL;
+ if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) {
+ ip_match = "ip4";
+ } else {
+ ip_match = "ip6";
+ }
+
+ /* Add logical flows to UNDNAT the load balanced reverse traffic in
+ * the router egress pipleine stage - S_ROUTER_OUT_UNDNAT if the logical
+ * router has a gateway router port associated.
+ */
+ struct ds undnat_match = DS_EMPTY_INITIALIZER;
+ ds_put_format(&undnat_match, "%s && (", ip_match);
+
+ for (size_t i = 0; i < lb_vip->n_backends; i++) {
+ struct ovn_lb_backend *backend = &lb_vip->backends[i];
+ ds_put_format(&undnat_match, "(%s.src == %s", ip_match,
+ backend->ip_str);
+
+ if (backend->port) {
+ ds_put_format(&undnat_match, " && %s.src == %d) || ",
+ proto, backend->port);
+ } else {
+ ds_put_cstr(&undnat_match, ") || ");
+ }
+ }
+ ds_chomp(&undnat_match, ' ');
+ ds_chomp(&undnat_match, '|');
+ ds_chomp(&undnat_match, '|');
+ ds_chomp(&undnat_match, ' ');
+
+ if (sset_contains(nat_entries, lb_vip->vip_str)) {
+ /* The load balancer vip is also present in the NAT entries.
+ * So add a high priority lflow to advance the the packet
+ * destined to the vip (and the vip port if defined)
+ * in the S_ROUTER_IN_UNSNAT stage.
+ * There seems to be an issue with ovs-vswitchd. When the new
+ * connection packet destined for the lb vip is received,
+ * it is dnat'ed in the S_ROUTER_IN_DNAT stage in the dnat
+ * conntrack zone. For the next packet, if it goes through
+ * unsnat stage, the conntrack flags are not set properly, and
+ * it doesn't hit the established state flows in
+ * S_ROUTER_IN_DNAT stage. */
+ struct ds unsnat_match = DS_EMPTY_INITIALIZER;
+ ds_put_format(&unsnat_match, "%s && %s.dst == %s && %s",
+ ip_match, ip_match, lb_vip->vip_str, proto);
+ if (lb_vip->vip_port) {
+ ds_put_format(&unsnat_match, " && %s.dst == %d", proto,
+ lb_vip->vip_port);
+ }
+
+ for (int i = 0; i < lb->n_nb_lr; i++) {
+ struct ovn_datapath *od = lb->nb_lr[i];
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_UNSNAT, 120,
+ ds_cstr(&unsnat_match), "next;",
+ &lb->nlb->header_);
+ }
+
+ ds_destroy(&unsnat_match);
+ }
+
for (size_t i = 0; i < lb->n_nb_lr; i++) {
struct ovn_datapath *od = lb->nb_lr[i];
char *new_match_p = new_match;
char *est_match_p = est_match;
+ char *est_actions = NULL;
if (od->l3redirect_port &&
(lb_vip->n_backends || !lb_vip->empty_backend_rej)) {
@@ -8939,12 +8916,11 @@ build_lrouter_nat_lflows_for_lb(struct ovn_lb_vip *lb_vip,
&lb->nlb->header_);
free(new_actions);
- char *est_actions = xasprintf("flags.force_snat_for_lb = 1; "
- "ct_dnat;");
+ est_actions = xasprintf("flags.force_snat_for_lb = 1; "
+ "ct_dnat;");
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
est_match_p, est_actions,
&lb->nlb->header_);
- free(est_actions);
} else {
ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, prio,
new_match_p, ds_cstr(&action),
@@ -8960,8 +8936,35 @@ build_lrouter_nat_lflows_for_lb(struct ovn_lb_vip *lb_vip,
if (est_match_p != est_match) {
free(est_match_p);
}
+
+ if (!od->l3dgw_port || !od->l3redirect_port || !lb_vip->n_backends) {
+ goto next;
+ }
+
+ char *undnat_match_p = xasprintf("%s) && outport == %s && "
+ "is_chassis_resident(%s)",
+ ds_cstr(&undnat_match),
+ od->l3dgw_port->json_key,
+ od->l3redirect_port->json_key);
+ if (snat_type == SKIP_SNAT) {
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
+ undnat_match_p, skip_snat_est_action,
+ &lb->nlb->header_);
+ } else if (snat_type == FORCE_SNAT) {
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
+ undnat_match_p, est_actions,
+ &lb->nlb->header_);
+ } else {
+ ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120,
+ undnat_match_p, "ct_dnat;",
+ &lb->nlb->header_);
+ }
+ free(undnat_match_p);
+next:
+ free(est_actions);
}
+ ds_destroy(&undnat_match);
ds_destroy(&action);
ds_destroy(&match);
@@ -8973,8 +8976,9 @@ build_lrouter_nat_lflows_for_lb(struct ovn_lb_vip *lb_vip,
static void
build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
- struct shash *meter_groups, struct ds *match,
- struct ds *action)
+ struct shash *meter_groups,
+ struct sset *nat_entries,
+ struct ds *match, struct ds *action)
{
if (!lb->n_nb_lr) {
return;
@@ -8983,7 +8987,8 @@ build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
for (size_t i = 0; i < lb->n_vips; i++) {
struct ovn_lb_vip *lb_vip = &lb->vips[i];
- build_lrouter_nat_lflows_for_lb(lb_vip, lb, &lb->vips_nb[i], lflows);
+ build_lrouter_nat_lflows_for_lb(lb_vip, lb, &lb->vips_nb[i],
+ lflows, nat_entries);
if (!build_empty_lb_event_flow(lb_vip, lb->nlb, meter_groups,
match, action)) {
@@ -8995,17 +9000,21 @@ build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap *lflows,
&lb->nlb->header_);
}
}
+
+ if (smap_get_bool(&lb->nlb->options, "skip_snat", false)) {
+ for (size_t i = 0; i < lb->n_nb_lr; i++) {
+ ovn_lflow_add(lflows, lb->nb_lr[i], S_ROUTER_OUT_SNAT, 120,
+ "flags.skip_snat_for_lb == 1 && ip", "next;");
+ }
+ }
}
static void
build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od,
- struct hmap *lbs, struct sset *nat_entries,
- struct ds *match, struct ds *actions)
+ struct hmap *lbs, struct ds *match)
{
/* A set to hold all ips that need defragmentation and tracking. */
struct sset all_ips = SSET_INITIALIZER(&all_ips);
- bool lb_force_snat_ip =
- !lport_addresses_is_empty(&od->lb_force_snat_addrs);
for (int i = 0; i < od->nbr->n_load_balancer; i++) {
struct nbrec_load_balancer *nb_lb = od->nbr->load_balancer[i];
@@ -9013,18 +9022,8 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od,
ovn_northd_lb_find(lbs, &nb_lb->header_.uuid);
ovs_assert(lb);
- enum lb_snat_type snat_type = NO_FORCE_SNAT;
- if (smap_get_bool(&nb_lb->options, "skip_snat", false)) {
- ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120,
- "flags.skip_snat_for_lb == 1 && ip", "next;");
- snat_type = SKIP_SNAT;
- } else if (lb_force_snat_ip || od->lb_force_snat_router_ip) {
- snat_type = FORCE_SNAT;
- }
-
for (size_t j = 0; j < lb->n_vips; j++) {
struct ovn_lb_vip *lb_vip = &lb->vips[j];
- ds_clear(actions);
if (!sset_contains(&all_ips, lb_vip->vip_str)) {
sset_add(&all_ips, lb_vip->vip_str);
@@ -9048,38 +9047,6 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od,
100, ds_cstr(match), "ct_next;",
&nb_lb->header_);
}
-
- /* Higher priority rules are added for load-balancing in DNAT
- * table. For every match (on a VIP[:port]), we add two flows
- * via add_router_lb_flow(). One flow is for specific matching
- * on ct.new with an action of "ct_lb($targets);". The other
- * flow is for ct.est with an action of "ct_dnat;". */
- ds_clear(match);
- if (IN6_IS_ADDR_V4MAPPED(&lb_vip->vip)) {
- ds_put_format(match, "ip && ip4.dst == %s",
- lb_vip->vip_str);
- } else {
- ds_put_format(match, "ip && ip6.dst == %s",
- lb_vip->vip_str);
- }
-
- bool is_udp = nullable_string_is_equal(nb_lb->protocol, "udp");
- bool is_sctp = nullable_string_is_equal(nb_lb->protocol,
- "sctp");
- const char *proto = is_udp ? "udp" : is_sctp ? "sctp" : "tcp";
-
- if (lb_vip->vip_port) {
- ds_put_format(match, " && %s && %s.dst == %d", proto,
- proto, lb_vip->vip_port);
- }
-
- if (od->l3redirect_port &&
- (lb_vip->n_backends || !lb_vip->empty_backend_rej)) {
- ds_put_format(match, " && is_chassis_resident(%s)",
- od->l3redirect_port->json_key);
- }
- add_router_lb_flow(lflows, od, snat_type, lb_vip, proto, nb_lb,
- nat_entries);
}
}
sset_destroy(&all_ips);
@@ -11828,6 +11795,7 @@ static void
build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
struct hmap *lflows,
struct hmap *lbs,
+ struct sset *nat_entries,
struct ds *match, struct ds *actions)
{
if (!od->nbr) {
@@ -11855,8 +11823,6 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
return;
}
- struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
-
bool dnat_force_snat_ip =
!lport_addresses_is_empty(&od->dnat_force_snat_addrs);
bool lb_force_snat_ip =
@@ -11883,7 +11849,7 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
/* ARP resolve for NAT IPs. */
if (od->l3dgw_port) {
- if (!sset_contains(&nat_entries, nat->external_ip)) {
+ if (!sset_contains(nat_entries, nat->external_ip)) {
ds_clear(match);
ds_put_format(
match, "outport == %s && %s == %s",
@@ -11900,13 +11866,13 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
100, ds_cstr(match),
ds_cstr(actions),
&nat->header_);
- sset_add(&nat_entries, nat->external_ip);
+ sset_add(nat_entries, nat->external_ip);
}
} else {
/* Add the NAT external_ip to the nat_entries even for
* gateway routers. This is required for adding load balancer
* flows.*/
- sset_add(&nat_entries, nat->external_ip);
+ sset_add(nat_entries, nat->external_ip);
}
/* S_ROUTER_OUT_UNDNAT */
@@ -12025,13 +11991,10 @@ build_lrouter_nat_defrag_and_lb(struct ovn_datapath *od,
/* Load balancing and packet defrag are only valid on
* Gateway routers or router with gateway port. */
if (!smap_get(&od->nbr->options, "chassis") && !od->l3dgw_port) {
- sset_destroy(&nat_entries);
return;
}
- build_lrouter_lb_flows(lflows, od, lbs, &nat_entries, match, actions);
-
- sset_destroy(&nat_entries);
+ build_lrouter_lb_flows(lflows, od, lbs, match);
}
@@ -12046,6 +12009,7 @@ struct lswitch_flow_build_info {
struct shash *meter_groups;
struct hmap *lbs;
struct hmap *bfd_connections;
+ struct sset *nat_entries;
char *svc_check_match;
struct ds match;
struct ds actions;
@@ -12095,7 +12059,8 @@ build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,
&lsi->actions);
build_misc_local_traffic_drop_flows_for_lrouter(od, lsi->lflows);
build_lrouter_arp_nd_for_datapath(od, lsi->lflows);
- build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->lbs, &lsi->match,
+ build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->lbs,
+ lsi->nat_entries, &lsi->match,
&lsi->actions);
}
@@ -12206,6 +12171,7 @@ build_lflows_thread(void *arg)
&lsi->actions);
build_lrouter_flows_for_lb(lb, lsi->lflows,
lsi->meter_groups,
+ lsi->nat_entries,
&lsi->match, &lsi->actions);
}
}
@@ -12273,7 +12239,8 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
struct hmap *mcgroups,
struct hmap *igmp_groups,
struct shash *meter_groups, struct hmap *lbs,
- struct hmap *bfd_connections)
+ struct hmap *bfd_connections,
+ struct sset *nat_entries)
{
char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);
@@ -12317,6 +12284,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
lsiv[index].meter_groups = meter_groups;
lsiv[index].lbs = lbs;
lsiv[index].bfd_connections = bfd_connections;
+ lsiv[index].nat_entries = nat_entries;
lsiv[index].svc_check_match = svc_check_match;
ds_init(&lsiv[index].match);
ds_init(&lsiv[index].actions);
@@ -12352,6 +12320,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
.meter_groups = meter_groups,
.lbs = lbs,
.bfd_connections = bfd_connections,
+ .nat_entries = nat_entries,
.svc_check_match = svc_check_match,
.match = DS_EMPTY_INITIALIZER,
.actions = DS_EMPTY_INITIALIZER,
@@ -12371,7 +12340,8 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
&lsi.actions,
&lsi.match);
build_lrouter_flows_for_lb(lb, lsi.lflows, lsi.meter_groups,
- &lsi.match, &lsi.actions);
+ lsi.nat_entries, &lsi.match,
+ &lsi.actions);
}
HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) {
build_lswitch_ip_mcast_igmp_mld(igmp_group,
@@ -12464,7 +12434,8 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
struct hmap *ports, struct hmap *port_groups,
struct hmap *mcgroups, struct hmap *igmp_groups,
struct shash *meter_groups,
- struct hmap *lbs, struct hmap *bfd_connections)
+ struct hmap *lbs, struct hmap *bfd_connections,
+ struct sset *nat_entries)
{
struct hmap lflows;
@@ -12475,7 +12446,7 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
build_lswitch_and_lrouter_flows(datapaths, ports,
port_groups, &lflows, mcgroups,
igmp_groups, meter_groups, lbs,
- bfd_connections);
+ bfd_connections, nat_entries);
if (hmap_count(&lflows) > max_seen_lflow_size) {
max_seen_lflow_size = hmap_count(&lflows);
@@ -13407,6 +13378,7 @@ ovnnb_db_run(struct northd_context *ctx,
struct shash meter_groups = SHASH_INITIALIZER(&meter_groups);
struct hmap lbs;
struct hmap bfd_connections = HMAP_INITIALIZER(&bfd_connections);
+ struct sset nat_entries = SSET_INITIALIZER(&nat_entries);
/* Sync ipsec configuration.
* Copy nb_cfg from northbound to southbound database.
@@ -13498,7 +13470,8 @@ ovnnb_db_run(struct northd_context *ctx,
build_meter_groups(ctx, &meter_groups);
build_bfd_table(ctx, &bfd_connections, ports);
build_lflows(ctx, datapaths, ports, &port_groups, &mcast_groups,
- &igmp_groups, &meter_groups, &lbs, &bfd_connections);
+ &igmp_groups, &meter_groups, &lbs, &bfd_connections,
+ &nat_entries);
ovn_update_ipv6_prefix(ports);
sync_address_sets(ctx);
@@ -13530,6 +13503,7 @@ ovnnb_db_run(struct northd_context *ctx,
hmap_destroy(&mcast_groups);
hmap_destroy(&port_groups);
hmap_destroy(&bfd_connections);
+ sset_destroy(&nat_entries);
struct shash_node *node, *next;
SHASH_FOR_EACH_SAFE (node, next, &meter_groups) {
--
2.31.1
More information about the dev
mailing list