@@ -1942,7 +1942,7 @@ add_lb_vip_hairpin_flows(struct ovn_controller_lb *lb,
struct ovn_lb_vip *lb_vip,
struct ovn_lb_backend *lb_backend,
uint8_t lb_proto,
- bool check_ct_label_for_lb_hairpin,
+ bool use_ct_mark,
struct ovn_desired_flow_table *flow_table)
{
uint64_t stub[1024 / 8];
@@ -2033,18 +2033,19 @@ add_lb_vip_hairpin_flows(struct ovn_controller_lb *lb,
* - packets must have ip.src == ip.dst at this point.
* - the destination protocol and port must be of a valid backend that
* has the same IP as ip.dst.
+ *
+ * During upgrades logical flows might still use the old way of storing
+ * ct.natted in ct_label. For backwards compatibility, only use ct_mark
+ * if ovn-northd notified ovn-controller to do that.
*/
- uint32_t lb_ct_mark = OVN_CT_NATTED;
- match_set_ct_mark_masked(&hairpin_match, lb_ct_mark, lb_ct_mark);
-
- ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
- lb->slb->header_.uuid.parts[0], &hairpin_match,
- &ofpacts, &lb->slb->header_.uuid);
+ if (use_ct_mark) {
+ uint32_t lb_ct_mark = OVN_CT_NATTED;
+ match_set_ct_mark_masked(&hairpin_match, lb_ct_mark, lb_ct_mark);
- /* The below flow is identical to the above except that it checks
- * ct_label.natted instead of ct_mark.natted, for backward compatibility
- * during the upgrade from a previous version that uses ct_label. */
- if (check_ct_label_for_lb_hairpin) {
+ ofctrl_add_flow(flow_table, OFTABLE_CHK_LB_HAIRPIN, 100,
+ lb->slb->header_.uuid.parts[0], &hairpin_match,
+ &ofpacts, &lb->slb->header_.uuid);
+ } else {
match_set_ct_mark_masked(&hairpin_match, 0, 0);
ovs_u128 lb_ct_label = {
.u64.lo = OVN_CT_NATTED,
@@ -2328,7 +2329,7 @@ add_lb_ct_snat_hairpin_flows(struct ovn_controller_lb *lb,
static void
consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb,
const struct hmap *local_datapaths,
- bool check_ct_label_for_lb_hairpin,
+ bool use_ct_mark,
struct ovn_desired_flow_table *flow_table,
struct simap *ids)
{
@@ -2368,8 +2369,7 @@ consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb,
struct ovn_lb_backend *lb_backend = &lb_vip->backends[j];
add_lb_vip_hairpin_flows(lb, lb_vip, lb_backend, lb_proto,
- check_ct_label_for_lb_hairpin,
- flow_table);
+ use_ct_mark, flow_table);
}
}
@@ -2382,8 +2382,7 @@ consider_lb_hairpin_flows(const struct sbrec_load_balancer *sbrec_lb,
* backends to handle the load balanced hairpin traffic. */
static void
add_lb_hairpin_flows(const struct sbrec_load_balancer_table *lb_table,
- const struct hmap *local_datapaths,
- bool check_ct_label_for_lb_hairpin,
+ const struct hmap *local_datapaths, bool use_ct_mark,
struct ovn_desired_flow_table *flow_table,
struct simap *ids,
struct id_pool *pool)
@@ -2406,8 +2405,7 @@ add_lb_hairpin_flows(const struct sbrec_load_balancer_table *lb_table,
ovs_assert(id_pool_alloc_id(pool, &id));
simap_put(ids, lb->name, id);
}
- consider_lb_hairpin_flows(lb, local_datapaths,
- check_ct_label_for_lb_hairpin,
+ consider_lb_hairpin_flows(lb, local_datapaths, use_ct_mark,
flow_table, ids);
}
}
@@ -2545,7 +2543,7 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct lflow_ctx_out *l_ctx_out)
l_ctx_in->local_datapaths,
l_ctx_out->flow_table);
add_lb_hairpin_flows(l_ctx_in->lb_table, l_ctx_in->local_datapaths,
- l_ctx_in->check_ct_label_for_lb_hairpin,
+ l_ctx_in->lb_hairpin_use_ct_mark,
l_ctx_out->flow_table,
l_ctx_out->hairpin_lb_ids,
l_ctx_out->hairpin_id_pool);
@@ -2709,7 +2707,7 @@ lflow_add_flows_for_datapath(const struct sbrec_datapath_binding *dp,
* associated. */
for (size_t i = 0; i < n_dp_lbs; i++) {
consider_lb_hairpin_flows(dp_lbs[i], l_ctx_in->local_datapaths,
- l_ctx_in->check_ct_label_for_lb_hairpin,
+ l_ctx_in->lb_hairpin_use_ct_mark,
l_ctx_out->flow_table,
l_ctx_out->hairpin_lb_ids);
}
@@ -2840,7 +2838,7 @@ lflow_handle_changed_lbs(struct lflow_ctx_in *l_ctx_in,
VLOG_DBG("Add load balancer hairpin flows for "UUID_FMT,
UUID_ARGS(&lb->header_.uuid));
consider_lb_hairpin_flows(lb, l_ctx_in->local_datapaths,
- l_ctx_in->check_ct_label_for_lb_hairpin,
+ l_ctx_in->lb_hairpin_use_ct_mark,
l_ctx_out->flow_table,
l_ctx_out->hairpin_lb_ids);
}
@@ -160,7 +160,7 @@ struct lflow_ctx_in {
const struct sset *related_lport_ids;
const struct shash *binding_lports;
const struct hmap *chassis_tunnels;
- bool check_ct_label_for_lb_hairpin;
+ bool lb_hairpin_use_ct_mark;
};
struct lflow_ctx_out {
@@ -131,6 +131,9 @@ static const char *ssl_ca_cert_file;
#define DEFAULT_LFLOW_CACHE_WMARK_PERC 50
#define DEFAULT_LFLOW_CACHE_TRIM_TO_MS 30000
+/* SB Global options defaults. */
+#define DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK false
+
struct controller_engine_ctx {
struct lflow_cache *lflow_cache;
struct if_status_mgr *if_mgr;
@@ -486,13 +489,6 @@ get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table)
return chassis_id;
}
-static bool
-get_check_ct_label_for_lb_hairpin(const char *northd_internal_ver)
-{
- unsigned int minor = ovn_parse_internal_version_minor(northd_internal_ver);
- return (minor <= 3);
-}
-
static void
update_ssl_config(const struct ovsrec_ssl_table *ssl_table)
{
@@ -2292,60 +2288,58 @@ non_vif_data_ovs_iface_handler(struct engine_node *node, void *data OVS_UNUSED)
return local_nonvif_data_handle_ovs_iface_changes(iface_table);
}
-struct ed_type_northd_internal_version {
- char *ver;
+struct ed_type_northd_options {
+ bool lb_hairpin_use_ct_mark;
};
static void *
-en_northd_internal_version_init(struct engine_node *node OVS_UNUSED,
- struct engine_arg *arg OVS_UNUSED)
+en_northd_options_init(struct engine_node *node OVS_UNUSED,
+ struct engine_arg *arg OVS_UNUSED)
{
- struct ed_type_northd_internal_version *n_ver = xzalloc(sizeof *n_ver);
- n_ver->ver = xstrdup("");
- return n_ver;
+ struct ed_type_northd_options *n_opts = xzalloc(sizeof *n_opts);
+ return n_opts;
}
static void
-en_northd_internal_version_cleanup(void *data)
+en_northd_options_cleanup(void *data OVS_UNUSED)
{
- struct ed_type_northd_internal_version *n_ver = data;
- free(n_ver->ver);
}
static void
-en_northd_internal_version_run(struct engine_node *node, void *data)
+en_northd_options_run(struct engine_node *node, void *data)
{
- struct ed_type_northd_internal_version *n_ver = data;
- struct sbrec_sb_global_table *sb_global_table =
- (struct sbrec_sb_global_table *)EN_OVSDB_GET(
- engine_get_input("SB_sb_global", node));
+ struct ed_type_northd_options *n_opts = data;
+ const struct sbrec_sb_global_table *sb_global_table =
+ EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
const struct sbrec_sb_global *sb_global =
sbrec_sb_global_table_first(sb_global_table);
- free(n_ver->ver);
- n_ver->ver =
- xstrdup(sb_global ? smap_get_def(&sb_global->options,
- "northd_internal_version", "") : "");
+
+ n_opts->lb_hairpin_use_ct_mark =
+ sb_global
+ ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
+ DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
+ : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
engine_set_node_state(node, EN_UPDATED);
}
static bool
-en_northd_internal_version_sb_sb_global_handler(struct engine_node *node,
- void *data)
+en_northd_options_sb_sb_global_handler(struct engine_node *node, void *data)
{
- struct ed_type_northd_internal_version *n_ver = data;
- struct sbrec_sb_global_table *sb_global_table =
- (struct sbrec_sb_global_table *)EN_OVSDB_GET(
- engine_get_input("SB_sb_global", node));
+ struct ed_type_northd_options *n_opts = data;
+ const struct sbrec_sb_global_table *sb_global_table =
+ EN_OVSDB_GET(engine_get_input("SB_sb_global", node));
const struct sbrec_sb_global *sb_global =
sbrec_sb_global_table_first(sb_global_table);
- const char *new_ver =
- sb_global ? smap_get_def(&sb_global->options,
- "northd_internal_version", "") : "";
- if (strcmp(new_ver, n_ver->ver)) {
- free(n_ver->ver);
- n_ver->ver = xstrdup(new_ver);
+ bool lb_hairpin_use_ct_mark =
+ sb_global
+ ? smap_get_bool(&sb_global->options, "lb_hairpin_use_ct_mark",
+ DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK)
+ : DEFAULT_SB_GLOBAL_LB_HAIRPIN_USE_CT_MARK;
+
+ if (lb_hairpin_use_ct_mark != n_opts->lb_hairpin_use_ct_mark) {
+ n_opts->lb_hairpin_use_ct_mark = lb_hairpin_use_ct_mark;
engine_set_node_state(node, EN_UPDATED);
}
return true;
@@ -2496,9 +2490,8 @@ init_lflow_ctx(struct engine_node *node,
engine_get_input_data("port_groups", node);
struct shash *port_groups = &pg_data->port_groups_cs_local;
- struct ed_type_northd_internal_version *n_ver =
- engine_get_input_data("northd_internal_version", node);
- ovs_assert(n_ver);
+ struct ed_type_northd_options *n_opts =
+ engine_get_input_data("northd_options", node);
l_ctx_in->sbrec_multicast_group_by_name_datapath =
sbrec_mc_group_by_name_dp;
@@ -2529,8 +2522,7 @@ init_lflow_ctx(struct engine_node *node,
l_ctx_in->related_lport_ids = &rt_data->related_lports.lport_ids;
l_ctx_in->binding_lports = &rt_data->lbinding_data.lports;
l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
- l_ctx_in->check_ct_label_for_lb_hairpin =
- get_check_ct_label_for_lb_hairpin(n_ver->ver);
+ l_ctx_in->lb_hairpin_use_ct_mark = n_opts->lb_hairpin_use_ct_mark;
l_ctx_out->flow_table = &fo->flow_table;
l_ctx_out->group_table = &fo->group_table;
@@ -3458,7 +3450,7 @@ main(int argc, char *argv[])
ENGINE_NODE(flow_output, "flow_output");
ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets");
ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups");
- ENGINE_NODE(northd_internal_version, "northd_internal_version");
+ ENGINE_NODE(northd_options, "northd_options");
#define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
SB_NODES
@@ -3507,10 +3499,10 @@ main(int argc, char *argv[])
engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL);
engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL);
- engine_add_input(&en_northd_internal_version, &en_sb_sb_global,
- en_northd_internal_version_sb_sb_global_handler);
+ engine_add_input(&en_northd_options, &en_sb_sb_global,
+ en_northd_options_sb_sb_global_handler);
- engine_add_input(&en_lflow_output, &en_northd_internal_version, NULL);
+ engine_add_input(&en_lflow_output, &en_northd_options, NULL);
/* Keep en_addr_sets before en_runtime_data because
* lflow_output_runtime_data_handler may *partially* reprocess a lflow when
@@ -15252,15 +15252,6 @@ ovnnb_db_run(struct northd_input *input_data,
if (!nb) {
nb = nbrec_nb_global_insert(ovnnb_txn);
}
- const struct sbrec_sb_global *sb = sbrec_sb_global_table_first(
- input_data->sbrec_sb_global_table);
- if (!sb) {
- sb = sbrec_sb_global_insert(ovnsb_txn);
- }
- if (nb->ipsec != sb->ipsec) {
- sbrec_sb_global_set_ipsec(sb, nb->ipsec);
- }
- sbrec_sb_global_set_options(sb, &nb->options);
const char *mac_addr_prefix = set_mac_prefix(smap_get(&nb->options,
"mac_prefix"));
@@ -15306,8 +15297,6 @@ ovnnb_db_run(struct northd_input *input_data,
nbrec_nb_global_set_options(nb, &options);
}
- smap_destroy(&options);
-
bool old_use_ldp = use_logical_dp_groups;
use_logical_dp_groups = smap_get_bool(&nb->options,
"use_logical_dp_groups", true);
@@ -15351,6 +15340,26 @@ ovnnb_db_run(struct northd_input *input_data,
sync_dns_entries(input_data, ovnsb_txn, &data->datapaths);
cleanup_stale_fdb_entries(input_data, &data->datapaths);
stopwatch_stop(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
+
+ /* Set up SB_Global (depends on chassis features). */
+ const struct sbrec_sb_global *sb = sbrec_sb_global_table_first(
+ input_data->sbrec_sb_global_table);
+ if (!sb) {
+ sb = sbrec_sb_global_insert(ovnsb_txn);
+ }
+ if (nb->ipsec != sb->ipsec) {
+ sbrec_sb_global_set_ipsec(sb, nb->ipsec);
+ }
+
+ /* Inform ovn-controllers whether LB flows will use ct_mark
+ * (i.e., only if all chassis support it).
+ */
+ smap_replace(&options, "lb_hairpin_use_ct_mark",
+ data->features.ct_lb_mark ? "true" : "false");
+ if (!smap_equal(&sb->options, &options)) {
+ sbrec_sb_global_set_options(sb, &options);
+ }
+ smap_destroy(&options);
}
/* Stores the list of chassis which references an ha_chassis_group.
@@ -199,6 +199,22 @@
tunnel interfaces.
</column>
</group>
+
+ <group title="Options for configuring Load Balancers">
+ <p>
+ These options apply when <code>ovn-controller</code> configures
+ load balancer related flows.
+ </p>
+
+ <column name="options" key="lb_hairpin_use_ct_mark">
+ This value is automatically set to <code>true</code> by
+ <code>ovn-northd</code> when action <code>ct_lb_mark</code> is used
+ for new load balancer sessions. <code>ovn-controller</code> then
+ knows that it should check <code>ct_mark.natted</code> to detect
+ load balanced traffic.
+ </column>
+ </group>
+
</group>
<group title="Connection Options">
@@ -2058,7 +2058,7 @@ AT_CHECK([echo $(($reprocess_count_new - $reprocess_count_old))], [0], [2
OVN_CLEANUP([hv1])
AT_CLEANUP
-AT_SETUP([ovn-controller - I-P handle northd_internal_version change])
+AT_SETUP([ovn-controller - I-P handle lb_hairpin_use_ct_mark change])
ovn_start --backup-northd=none
@@ -2089,8 +2089,8 @@ lflow_run_new=$(read_counter lflow_run)
AT_CHECK([echo $(($lflow_run_new - $lflow_run_old))], [0], [0
])
-# northd_internal_version update in sb_global:options should trigger lflow_run.
-# The below steps should cause northd_internal_version change twice. One by
+# lb_hairpin_use_ct_mark update in sb_global:options should trigger lflow_run.
+# The below steps should cause lb_hairpin_use_ct_mark change twice. One by
# ovn-sbctl, and the other by ovn-northd to change it back.
# In some cases, both changes are catched by ovn-controller in the same run,
@@ -2098,7 +2098,7 @@ AT_CHECK([echo $(($lflow_run_new - $lflow_run_old))], [0], [0
OVS_WAIT_UNTIL([
lflow_run_old=$(read_counter lflow_run)
- check ovn-sbctl set SB_Global . options:northd_internal_version=foo
+ check ovn-sbctl set SB_Global . options:lb_hairpin_use_ct_mark=false
check ovn-nbctl --wait=hv sync
lflow_run_new=$(read_counter lflow_run)
test x"$(($lflow_run_new - $lflow_run_old))" = x2
Change the way ovn-controller decides whether it should match on ct_mark.natted or ct_label.natted for hairpin load balancer traffic. Until now this was done solely based on the northd-reported internal version. However, to cover the case when OVN central components are not the last ones to be updated, ovn-northd now explicitly informs ovn-controller whether it should use ct_mark or ct_label.natted via a new option in the OVN_Southbound.SB_Global record: lb_hairpin_use_ct_mark. Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- controller/lflow.c | 40 ++++++++++---------- controller/lflow.h | 2 + controller/ovn-controller.c | 86 ++++++++++++++++++++----------------------- northd/northd.c | 31 ++++++++++------ ovn-sb.xml | 16 ++++++++ tests/ovn-controller.at | 8 ++-- 6 files changed, 99 insertions(+), 84 deletions(-)