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." _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
