The registers used in actions and lflow created by controller are
hardcoded, which causes upgrade issues. When the register are changed
the "old" version might be still propagated by northd until upgraded.
To prevent that add flag that will be populated only by northd that
is using the "new" version of registers. The controller checks
this flag and uses proper registers in lflows/actions.
Reported-by: Xavier Simonart <xsimo...@redhat.com>
Suggested-by: Dumitru Ceara <dce...@redhat.com>
Signed-off-by: Ales Musil <amu...@redhat.com>
---
controller/lflow.c | 52 +++++++++++++++++++++++-------------
controller/lflow.h | 1 +
controller/ovn-controller.c | 19 +++++++++++++
include/ovn/actions.h | 4 +++
include/ovn/logical-fields.h | 7 +++++
lib/actions.c | 10 +++++--
northd/en-global-config.c | 4 +++
tests/test-ovn.c | 1 +
8 files changed, 78 insertions(+), 20 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index 856261f40..a77d07d22 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -100,7 +100,8 @@ consider_logical_flow(const struct sbrec_logical_flow
*lflow,
static void
consider_lb_hairpin_flows(const struct ovn_controller_lb *lb,
const struct hmap *local_datapaths,
- struct ovn_desired_flow_table *flow_table);
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation);
static void add_port_sec_flows(const struct shash *binding_lports,
const struct sbrec_chassis *,
@@ -878,6 +879,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow
*lflow,
.lflow_uuid = lflow->header_.uuid,
.dp_key = ldp->datapath->tunnel_key,
.explicit_arp_ns_output = l_ctx_in->explicit_arp_ns_output,
+ .register_consolidation = l_ctx_in->register_consolidation,
.pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
.ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
@@ -1645,7 +1647,8 @@ static void
add_lb_vip_hairpin_flows(const struct ovn_controller_lb *lb,
struct ovn_lb_vip *lb_vip,
struct ovn_lb_backend *lb_backend,
- struct ovn_desired_flow_table *flow_table)
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation)
{
uint64_t stub[1024 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
@@ -1677,9 +1680,10 @@ add_lb_vip_hairpin_flows(const struct ovn_controller_lb
*lb,
if (!lb->hairpin_orig_tuple) {
match_set_ct_nw_dst(&hairpin_match, vip4);
} else {
- match_set_reg(&hairpin_match,
- MFF_LOG_LB_ORIG_DIP_IPV4 - MFF_LOG_REG0,
- ntohl(vip4));
+ enum mf_field_id id = register_consolidation
+ ? MFF_LOG_LB_ORIG_DIP_IPV4
+ : MFF_LOG_LB_ORIG_DIP_IPV4_OLD;
+ match_set_reg(&hairpin_match, id - MFF_LOG_REG0, ntohl(vip4));
}
add_lb_vip_hairpin_reply_action(NULL, snat_vip4, lb->proto,
@@ -1788,7 +1792,8 @@ static void
add_lb_ct_snat_hairpin_vip_flow(const struct ovn_controller_lb *lb,
const struct ovn_lb_vip *lb_vip,
const struct hmap *local_datapaths,
- struct ovn_desired_flow_table *flow_table)
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation)
{
uint64_t stub[1024 / 8];
struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(stub);
@@ -1845,8 +1850,10 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
if (!lb->hairpin_orig_tuple) {
match_set_ct_nw_dst(&match, vip4);
} else {
- match_set_reg(&match, MFF_LOG_LB_ORIG_DIP_IPV4 - MFF_LOG_REG0,
- ntohl(vip4));
+ enum mf_field_id id = register_consolidation
+ ? MFF_LOG_LB_ORIG_DIP_IPV4
+ : MFF_LOG_LB_ORIG_DIP_IPV4_OLD;
+ match_set_reg(&match, id - MFF_LOG_REG0, ntohl(vip4));
}
} else {
match_set_dl_type(&match, htons(ETH_TYPE_IPV6));
@@ -1922,7 +1929,8 @@ add_lb_ct_snat_hairpin_vip_flow(const struct
ovn_controller_lb *lb,
static void
add_lb_ct_snat_hairpin_flows(const struct ovn_controller_lb *lb,
const struct hmap *local_datapaths,
- struct ovn_desired_flow_table *flow_table)
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation)
{
/* We must add a flow for each LB VIP. In the general case, this flow
is added to the OFTABLE_CT_SNAT_HAIRPIN table. If it matches, we
@@ -1956,14 +1964,15 @@ add_lb_ct_snat_hairpin_flows(const struct
ovn_controller_lb *lb,
for (int i = 0; i < lb->n_vips; i++) {
add_lb_ct_snat_hairpin_vip_flow(lb, &lb->vips[i], local_datapaths,
- flow_table);
+ flow_table, register_consolidation);
}
}
static void
consider_lb_hairpin_flows(const struct ovn_controller_lb *lb,
const struct hmap *local_datapaths,
- struct ovn_desired_flow_table *flow_table)
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation)
{
for (size_t i = 0; i < lb->n_vips; i++) {
struct ovn_lb_vip *lb_vip = &lb->vips[i];
@@ -1971,11 +1980,13 @@ consider_lb_hairpin_flows(const struct
ovn_controller_lb *lb,
for (size_t j = 0; j < lb_vip->n_backends; j++) {
struct ovn_lb_backend *lb_backend = &lb_vip->backends[j];
- add_lb_vip_hairpin_flows(lb, lb_vip, lb_backend, flow_table);
+ add_lb_vip_hairpin_flows(lb, lb_vip, lb_backend, flow_table,
+ register_consolidation);
}
}
- add_lb_ct_snat_hairpin_flows(lb, local_datapaths, flow_table);
+ add_lb_ct_snat_hairpin_flows(lb, local_datapaths, flow_table,
+ register_consolidation);
}
/* Adds OpenFlow flows to flow tables for each Load balancer VIPs and
@@ -1983,11 +1994,13 @@ consider_lb_hairpin_flows(const struct
ovn_controller_lb *lb,
static void
add_lb_hairpin_flows(const struct hmap *local_lbs,
const struct hmap *local_datapaths,
- struct ovn_desired_flow_table *flow_table)
+ struct ovn_desired_flow_table *flow_table,
+ bool register_consolidation)
{
const struct ovn_controller_lb *lb;
HMAP_FOR_EACH (lb, hmap_node, local_lbs) {
- consider_lb_hairpin_flows(lb, local_datapaths, flow_table);
+ consider_lb_hairpin_flows(lb, local_datapaths, flow_table,
+ register_consolidation);
}
}
@@ -2147,7 +2160,8 @@ lflow_run(struct lflow_ctx_in *l_ctx_in, struct
lflow_ctx_out *l_ctx_out)
l_ctx_out->flow_table);
add_lb_hairpin_flows(l_ctx_in->local_lbs,
l_ctx_in->local_datapaths,
- l_ctx_out->flow_table);
+ l_ctx_out->flow_table,
+ l_ctx_in->register_consolidation);
add_fdb_flows(l_ctx_in->fdb_table, l_ctx_in->local_datapaths,
l_ctx_out->flow_table,
l_ctx_in->sbrec_port_binding_by_key,
@@ -2401,7 +2415,8 @@ lflow_handle_changed_lbs(struct lflow_ctx_in *l_ctx_in,
UUID_FMT, UUID_ARGS(&uuid_node->uuid));
ofctrl_remove_flows(l_ctx_out->flow_table, &uuid_node->uuid);
consider_lb_hairpin_flows(lb, l_ctx_in->local_datapaths,
- l_ctx_out->flow_table);
+ l_ctx_out->flow_table,
+ l_ctx_in->register_consolidation);
}
UUIDSET_FOR_EACH (uuid_node, new_lbs) {
@@ -2410,7 +2425,8 @@ lflow_handle_changed_lbs(struct lflow_ctx_in *l_ctx_in,
VLOG_DBG("Add load balancer hairpin flows for "UUID_FMT,
UUID_ARGS(&uuid_node->uuid));
consider_lb_hairpin_flows(lb, l_ctx_in->local_datapaths,
- l_ctx_out->flow_table);
+ l_ctx_out->flow_table,
+ l_ctx_in->register_consolidation);
}
return true;
diff --git a/controller/lflow.h b/controller/lflow.h
index 206328f9e..93a9f3b7e 100644
--- a/controller/lflow.h
+++ b/controller/lflow.h
@@ -135,6 +135,7 @@ struct lflow_ctx_in {
bool localnet_learn_fdb;
bool localnet_learn_fdb_changed;
bool explicit_arp_ns_output;
+ bool register_consolidation;
};
struct lflow_ctx_out {
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 910963e25..63c787bde 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -3520,6 +3520,7 @@ struct ed_type_northd_options {
* switch (i.e with localnet port) should
* be tunnelled or sent via the localnet
* port. Default value is 'false'. */
+ bool register_consolidation;
};
@@ -3557,6 +3558,12 @@ en_northd_options_run(struct engine_node *node, void
*data)
false)
: false;
+ n_opts->register_consolidation =
+ sb_global
+ ? smap_get_bool(&sb_global->options, "register_consolidation",
+ false)
+ : false;
+
engine_set_node_state(node, EN_UPDATED);
}
@@ -3591,6 +3598,17 @@ en_northd_options_sb_sb_global_handler(struct
engine_node *node, void *data)
engine_set_node_state(node, EN_UPDATED);
}
+ bool register_consolidation =
+ sb_global
+ ? smap_get_bool(&sb_global->options, "register_consolidation",
+ false)
+ : false;
+
+ if (register_consolidation != n_opts->register_consolidation) {
+ n_opts->register_consolidation = register_consolidation;
+ engine_set_node_state(node, EN_UPDATED);
+ }
+
return true;
}
@@ -3820,6 +3838,7 @@ init_lflow_ctx(struct engine_node *node,
l_ctx_in->localnet_learn_fdb_changed = rt_data->localnet_learn_fdb_changed;
l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
l_ctx_in->explicit_arp_ns_output = n_opts->explicit_arp_ns_output;
+ l_ctx_in->register_consolidation = n_opts->register_consolidation;
l_ctx_in->nd_ra_opts = &fo->nd_ra_opts;
l_ctx_in->dhcp_opts = &dhcp_opts->v4_opts;
l_ctx_in->dhcpv6_opts = &dhcp_opts->v6_opts;
diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index db7342f1d..3c3d06570 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -893,6 +893,10 @@ struct ovnact_encode_params {
/* Indication if we should add explicit output after arp/nd_ns action. */
bool explicit_arp_ns_output;
+ /* Indication if we should use the "old" version of registers. */
+ bool register_consolidation;
+
+
/* OVN maps each logical flow table (ltable), one-to-one, onto a physical
* OpenFlow flow table (ptable). A number of parameters describe this
* mapping and data related to flow tables:
diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
index 6a87fc386..f853b1f61 100644
--- a/include/ovn/logical-fields.h
+++ b/include/ovn/logical-fields.h
@@ -64,6 +64,13 @@ enum ovn_controller_event {
#define MFF_LOG_CT_ORIG_TP_DST_PORT MFF_REG2 /* REG_ORIG_TP_DPORT
* (bits 0..15). */
+/* Logical registers that are needed for backwards
+ * compatibility with older northd versions.
+ * XXX: All of them can be removed in 26.09. */
+#define MFF_LOG_LB_ORIG_DIP_IPV4_OLD MFF_REG1
+#define MFF_LOG_LB_AFF_MATCH_PORT_OLD MFF_REG8
+#define MFF_LOG_LB_AFF_MATCH_LS_IP6_ADDR_OLD MFF_XXREG0
+
void ovn_init_symtab(struct shash *symtab);
/* MFF_LOG_FLAGS_REG bit assignments */
diff --git a/lib/actions.c b/lib/actions.c
index 62e73b2c5..5dbcfe324 100644
--- a/lib/actions.c
+++ b/lib/actions.c
@@ -5371,7 +5371,10 @@ encode_COMMIT_LB_AFF(const struct ovnact_commit_lb_aff
*lb_aff,
imm_backend_ip = (union mf_value) {
.ipv6 = lb_aff->backend,
};
- ol_spec->dst.field = mf_from_id(MFF_LOG_LB_AFF_MATCH_IP6_ADDR);
+ enum mf_field_id id = !ep->register_consolidation && ep->is_switch
+ ? MFF_LOG_LB_AFF_MATCH_LS_IP6_ADDR_OLD
+ : MFF_LOG_LB_AFF_MATCH_IP6_ADDR;
+ ol_spec->dst.field = mf_from_id(id);
} else {
ovs_be32 ip4 = in6_addr_get_mapped_ipv4(&lb_aff->backend);
imm_backend_ip = (union mf_value) {
@@ -5399,7 +5402,10 @@ encode_COMMIT_LB_AFF(const struct ovnact_commit_lb_aff
*lb_aff,
.be16 = htons(lb_aff->backend_port),
};
- ol_spec->dst.field = mf_from_id(MFF_LOG_LB_AFF_MATCH_PORT);
+ enum mf_field_id id = !ep->register_consolidation
+ ? MFF_LOG_LB_AFF_MATCH_PORT_OLD
+ : MFF_LOG_LB_AFF_MATCH_PORT;
+ ol_spec->dst.field = mf_from_id(id);
ol_spec->dst_type = NX_LEARN_DST_LOAD;
ol_spec->src_type = NX_LEARN_SRC_IMMEDIATE;
ol_spec->dst.ofs = 0;
diff --git a/northd/en-global-config.c b/northd/en-global-config.c
index ce16c26f2..9aef9209f 100644
--- a/northd/en-global-config.c
+++ b/northd/en-global-config.c
@@ -590,6 +590,10 @@ update_sb_config_options_to_sbrec(struct
ed_type_global_config *config_data,
* arp/nd_ns action. */
smap_add(options, "arp_ns_explicit_output", "true");
+ /* Adds indication that northd has code with consolidated
+ * register usage. */
+ smap_add(options, "register_consolidation", "true");
+
if (!smap_equal(&sb->options, options)) {
sbrec_sb_global_set_options(sb, options);
}
diff --git a/tests/test-ovn.c b/tests/test-ovn.c
index b097ec084..c310e197e 100644
--- a/tests/test-ovn.c
+++ b/tests/test-ovn.c
@@ -1366,6 +1366,7 @@ test_parse_actions(struct ovs_cmdl_context *ctx
OVS_UNUSED)
.meter_table = &meter_table,
.collector_ids = &collector_ids,
.explicit_arp_ns_output = true,
+ .register_consolidation = true,
.pipeline = OVNACT_P_INGRESS,
.ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE,
--
2.48.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
