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.Load_Balancer record: hairpin_use_ct_mark.
Signed-off-by: Dumitru Ceara <[email protected]>
---
controller/lflow.c | 32 ++++++++++++--------------------
controller/lflow.h | 1 -
controller/ovn-controller.c | 9 ---------
lib/lb.c | 3 +++
lib/lb.h | 4 ++++
northd/northd.c | 8 ++++++--
ovn-sb.xml | 8 ++++++++
tests/ovn-northd.at | 2 +-
8 files changed, 34 insertions(+), 33 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 7a3419305..0d9184285 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -1942,7 +1942,6 @@ 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,
struct ovn_desired_flow_table *flow_table)
{
uint64_t stub[1024 / 8];
@@ -2033,18 +2032,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 flow 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 (lb->hairpin_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 +2328,6 @@ 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,
struct ovn_desired_flow_table *flow_table,
struct simap *ids)
{
@@ -2368,7 +2367,6 @@ 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);
}
}
@@ -2383,7 +2381,6 @@ consider_lb_hairpin_flows(const struct
sbrec_load_balancer *sbrec_lb,
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,
struct ovn_desired_flow_table *flow_table,
struct simap *ids,
struct id_pool *pool)
@@ -2406,9 +2403,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,
- flow_table, ids);
+ consider_lb_hairpin_flows(lb, local_datapaths, flow_table, ids);
}
}
@@ -2545,7 +2540,6 @@ 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_out->flow_table,
l_ctx_out->hairpin_lb_ids,
l_ctx_out->hairpin_id_pool);
@@ -2709,7 +2703,6 @@ 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_out->flow_table,
l_ctx_out->hairpin_lb_ids);
}
@@ -2840,7 +2833,6 @@ 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_out->flow_table,
l_ctx_out->hairpin_lb_ids);
}
diff --git a/controller/lflow.h b/controller/lflow.h
index ad9449d3a..2aa896a75 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -160,7 +160,6 @@ 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;
};
struct lflow_ctx_out {
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index b597c0e37..b9801c42d 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -486,13 +486,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)
{
@@ -2529,8 +2522,6 @@ 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_out->flow_table = &fo->flow_table;
l_ctx_out->group_table = &fo->group_table;
diff --git a/lib/lb.c b/lib/lb.c
index 7b0ed1abe..6a48175a7 100644
--- a/lib/lb.c
+++ b/lib/lb.c
@@ -304,6 +304,9 @@ ovn_controller_lb_create(const struct sbrec_load_balancer
*sbrec_lb)
lb->hairpin_orig_tuple = smap_get_bool(&sbrec_lb->options,
"hairpin_orig_tuple",
false);
+ lb->hairpin_use_ct_mark = smap_get_bool(&sbrec_lb->options,
+ "hairpin_use_ct_mark",
+ false);
ovn_lb_get_hairpin_snat_ip(&sbrec_lb->header_.uuid, &sbrec_lb->options,
&lb->hairpin_snat_ips);
return lb;
diff --git a/lib/lb.h b/lib/lb.h
index 832ed31fb..d703c6bf5 100644
--- a/lib/lb.h
+++ b/lib/lb.h
@@ -101,6 +101,10 @@ struct ovn_controller_lb {
bool hairpin_orig_tuple; /* True if ovn-northd stores the original
* destination tuple in registers.
*/
+ bool hairpin_use_ct_mark; /* True if ovn-northd uses ct_mark for
+ * load balancer sessions. False if it uses
+ * ct_label.
+ */
struct lport_addresses hairpin_snat_ips; /* IP (v4 and/or v6) to be used
* as source for hairpinned
diff --git a/northd/northd.c b/northd/northd.c
index 450e05ad6..6e2f2a880 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -4146,7 +4146,8 @@ build_lb_port_related_data(struct hmap *datapaths, struct
hmap *ports,
*/
static void
sync_lbs(struct northd_input *input_data, struct ovsdb_idl_txn *ovnsb_txn,
- struct hmap *datapaths, struct hmap *lbs)
+ struct hmap *datapaths, struct hmap *lbs,
+ const struct chassis_features *features)
{
struct ovn_northd_lb *lb;
@@ -4193,6 +4194,8 @@ sync_lbs(struct northd_input *input_data, struct ovsdb_idl_txn *ovnsb_txn,
struct smap options;
smap_clone(&options, &lb->nlb->options);
smap_replace(&options, "hairpin_orig_tuple", "true");
+ smap_replace(&options, "hairpin_use_ct_mark",
+ features->ct_lb_mark ? "true" : "false");
struct sbrec_datapath_binding **lb_dps =
xmalloc(lb->n_nb_ls * sizeof *lb_dps);
@@ -15344,7 +15347,8 @@ ovnnb_db_run(struct northd_input *input_data,
ovn_update_ipv6_options(&data->ports);
ovn_update_ipv6_prefix(&data->ports);
- sync_lbs(input_data, ovnsb_txn, &data->datapaths, &data->lbs);
+ sync_lbs(input_data, ovnsb_txn, &data->datapaths, &data->lbs,
+ &data->features);
sync_address_sets(input_data, ovnsb_txn, &data->datapaths);
sync_port_groups(input_data, ovnsb_txn, &data->port_groups);
sync_meters(input_data, ovnsb_txn, &data->meter_groups);
diff --git a/ovn-sb.xml b/ovn-sb.xml
index 2dc0d5bea..b8b8074ec 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -4566,6 +4566,14 @@ tcp.flags = RST;
of the load balanced packets are stored in registers
<code>reg1, reg2, xxreg1</code>.
</column>
+ <column name="options" key="hairpin_use_ct_mark"
+ type='{"type": "boolean"}'>
+ 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 title="Common Columns">
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 7bb6d33ac..4bb815c7b 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -2563,7 +2563,7 @@ check_column "" sb:datapath_binding load_balancers
external_ids:name=sw1
echo
echo "__file__:__line__: Set hairpin_snat_ip on lb1 and check that SB DB is
updated."
check ovn-nbctl --wait=sb set Load_Balancer lb1
options:hairpin_snat_ip="42.42.42.42 4242::4242"
-check_column "$lb1_uuid" sb:load_balancer _uuid name=lb1
options='{hairpin_orig_tuple="true", hairpin_snat_ip="42.42.42.42 4242::4242"}'
+check_column "$lb1_uuid" sb:load_balancer _uuid name=lb1 options='{hairpin_orig_tuple="true",
hairpin_snat_ip="42.42.42.42 4242::4242", hairpin_use_ct_mark="true"}'
echo
echo "__file__:__line__: Delete load balancers lb1 and lbg1 and check that
datapath sw1's load_balancers is still empty."
