On 6/2/22 16:35, Han Zhou wrote:
> On Thu, Jun 2, 2022 at 5:07 AM Dumitru Ceara <[email protected]> wrote:
>>
>> On 6/2/22 04:08, Han Zhou wrote:
>>> On Wed, Jun 1, 2022 at 11:44 AM Mark Michelson <[email protected]>
> wrote:
>>>>
>>>> On 6/1/22 13:44, Numan Siddique wrote:
>>>>> On Wed, Jun 1, 2022 at 12:52 PM Mark Michelson <[email protected]>
>>> wrote:
>>>>>>
>>>>>> Thanks Dumitru. This looks good to me.
>>>>>>
>>>>>> Acked-by: Mark Michelson <[email protected]>
>>>>>
>>>>> Thanks Dumitru.
>>>>> I applied this patch to main and branch-22.06.
>>>>>
>>>>> Looks like it's not applying cleanly to branch-22.03.
>>>>>
>>>>> Numan
>>>>
>>>> I pushed the changes to 21.12 and 22.03.
>>>>
>>> Sorry that I just noticed this change. The original ct_lb_mark change
>>> didn't consider the order that ovn-northd upgrades before
> ovn-controller.
>>> However, I think this change would cause another problem for this
> upgrade
>>> order.
>>
>> You're right, I forgot about this case.
>>
>>> Due to the direct usage of ct.label (rather than through an ovn action
> from
>>> logical flows) in ovn-controller generated LB hair-pin flows, there was
>>> already a compatibility issue between ovn-northd and ovn-controller
> during
>>> upgrade (pointed out by Numan during code review of the ct_lb_mark
> change),
>>> and that patch took care of the incompatibility by checking ovn-northd
>>> version string in ovn-controller:
>>>
>>> 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);
>>> }
>>>
>>> With this check, ovn-controller will decide to use ct.label or ct.mark
> for
>>> the LB hair-pin flows according to ovn-northd version, with the
> assumption
>>> that ovn-controller is upgraded before ovn-northd. But it didn't support
>>> the use case when ovn-northd is upgraded first.
>>> Now with this change, if ovn-northd is upgraded before ovn-controller,
> any
>>> upgraded ovn-controller would expect the new ovn-northd using the
>>> ct_lb_mark, which would be a problem for the hair-pin flows until all
> the
>>> ovn-controllers are upgraded.
>>
>> Sure but before the patch any ovn-northd-before-ovn-controller upgrade
>> would have broken all load balancer use cases. Now we handle most of
>> the traffic patterns, except hairpin.
>>
>> I'm working on a patch to handle the hairpin case when ovn-northd is
>> upgraded before all ovn-controllers are upgraded.
>>
>> While doing that I also noticed we need to use a similar approach for
>> ct_mark.blocked and ct_mark.ecmp_reply_port.
>>
>> I'll post a series to cover all this.
>>
> Hi Dumitru,
>
Hi Han,
> Thanks for working on this. I forgot to mention another problem that the
> fix that fixes upgrading problems may cause new upgrading problems for the
> users that have already upgraded to the last version.
> Assume both northd and ovn-controller are using the latest (ct mark)
> version of the code already before this patch, now they are upgrading to
> the version of this patch:
>
> 1. If northd is upgraded first: because not all ovn-controllers are
> upgraded, so it will use ct_lb instead of ct_lb_mark, causing a new
> incompatibility scenario for the hair-pin flows
> 2. If ovn-controller is upgraded first: no problem
>
> Maybe there are not many users that would encounter this, since the ct mark
> change was backported quite recently, but please take this into
> consideration (for both upgrading orders) if you attempt to address the
> rest of the upgrading problems for this.
I don't really see an easy way to fix this scenario. However, I don't
think this is a big concern. There's no official release including the
ct_mark change yet.
In my opinion things are OK if upgrades work fine (regardless of order
of components) from version A.X to version A.Y where:
- "A.X" is a stable release before ct_mark was used.
- "A.Y" is the most recent minor release of stable branch "A".
So as long as we fix this before the next set of stable releases I think
we're fine.
Regarding users: out of the 3 CMSs that we use downstream 2 already
ensure that ovn-controllers are upgraded before ovn-northd and DBs
(OpenStack and OpenShift). For RHEV that's not the case but AFAIK RHEV
doesn't need the logical switch Load_Balancer hairpin use case.
Thanks,
Dumitru
>
> Thanks,
> Han
>
>>>
>>> Thanks,
>>> Han
>>
>> Thanks,
>> Dumitru
>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> On 6/1/22 10:36, Dumitru Ceara wrote:
>>>>>>> Commit a075230e4a0f ("Use ct_mark for masked access to make flows
>>>>>>> HW-offloading friendly.") started using the ct_lb_mark action
> instead
>>> of
>>>>>>> ct_lb. In usual scenarios this new feature would be picked up when
>>> the
>>>>>>> next stable release becomes available (i.e., 22.06.0). However, the
>>>>>>> commit was also backported to stable branches (branch-22.03 and
>>>>>>> branch-21.12).
>>>>>>>
>>>>>>> While the supported upgrade scenario for OVN when moving to a new
>>>>>>> stable release is to ensure that ovn-controllers are upgraded first,
>>>>>>> it's not really clear that this restriction applies to "z-stream"
>>>>>>> upgrades too (e.g., from v21.12.1 to v21.12.2).
>>>>>>>
>>>>>>> Some CMSs, like RHEV (Red Hat Virtualization), expect
> ovn-controllers
>>>>>>> running older code from a stable branch to be able to interpret all
>>>>>>> Southbound contents generated by ovn-northd instances built from the
>>>>>>> same or newer versions of the stable branch.
>>>>>>>
>>>>>>> Ensure that ct_lb_mark is used only when all chassis registered in
> the
>>>>>>> Southbound support it.
>>>>>>>
>>>>>>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2091565
>>>>>>> Signed-off-by: Dumitru Ceara <[email protected]>
>>>>>>> ---
>>>>>>> NOTE: If accepted, this patch should be backported to branch-22.03
> and
>>>>>>> branch-21.12. The alternative is to revert a075230e4a0f and its
>>>>>>> dependents from all stable branches.
>>>>>>> ---
>>>>>>> controller/chassis.c | 6 +++
>>>>>>> include/ovn/features.h | 1 +
>>>>>>> northd/en-lflow.c | 1 +
>>>>>>> northd/northd.c | 112
>>> ++++++++++++++++++++++++++++-------------
>>>>>>> northd/northd.h | 6 +++
>>>>>>> tests/ovn-northd.at | 69 ++++++++++++++++++++++++-
>>>>>>> 6 files changed, 159 insertions(+), 36 deletions(-)
>>>>>>>
>>>>>>> diff --git a/controller/chassis.c b/controller/chassis.c
>>>>>>> index 8a1559653..239658461 100644
>>>>>>> --- a/controller/chassis.c
>>>>>>> +++ b/controller/chassis.c
>>>>>>> @@ -350,6 +350,7 @@ chassis_build_other_config(const struct
>>> ovs_chassis_cfg *ovs_cfg,
>>>>>>> smap_replace(config, "is-interconn",
>>>>>>> ovs_cfg->is_interconn ? "true" : "false");
>>>>>>> smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true");
>>>>>>> + smap_replace(config, OVN_FEATURE_CT_LB_MARK, "true");
>>>>>>> }
>>>>>>>
>>>>>>> /*
>>>>>>> @@ -455,6 +456,11 @@ chassis_other_config_changed(const struct
>>> ovs_chassis_cfg *ovs_cfg,
>>>>>>> return true;
>>>>>>> }
>>>>>>>
>>>>>>> + if (!smap_get_bool(&chassis_rec->other_config,
>>> OVN_FEATURE_CT_LB_MARK,
>>>>>>> + false)) {
>>>>>>> + return true;
>>>>>>> + }
>>>>>>> +
>>>>>>> return false;
>>>>>>> }
>>>>>>>
>>>>>>> diff --git a/include/ovn/features.h b/include/ovn/features.h
>>>>>>> index d12a8eb0d..09f002287 100644
>>>>>>> --- a/include/ovn/features.h
>>>>>>> +++ b/include/ovn/features.h
>>>>>>> @@ -22,6 +22,7 @@
>>>>>>>
>>>>>>> /* ovn-controller supported feature names. */
>>>>>>> #define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif"
>>>>>>> +#define OVN_FEATURE_CT_LB_MARK "ct-lb-mark"
>>>>>>>
>>>>>>> /* OVS datapath supported features. Based on availability OVN
>>> might generate
>>>>>>> * different types of openflows.
>>>>>>> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
>>>>>>> index ffbdaf4e8..fa0dfcbe0 100644
>>>>>>> --- a/northd/en-lflow.c
>>>>>>> +++ b/northd/en-lflow.c
>>>>>>> @@ -60,6 +60,7 @@ void en_lflow_run(struct engine_node *node, void
>>> *data OVS_UNUSED)
>>>>>>> lflow_input.meter_groups = &northd_data->meter_groups;
>>>>>>> lflow_input.lbs = &northd_data->lbs;
>>>>>>> lflow_input.bfd_connections = &northd_data->bfd_connections;
>>>>>>> + lflow_input.features = &northd_data->features;
>>>>>>> lflow_input.ovn_internal_version_changed =
>>>>>>> northd_data->ovn_internal_version_changed;
>>>>>>>
>>>>>>> diff --git a/northd/northd.c b/northd/northd.c
>>>>>>> index 51dec36b3..511ef6376 100644
>>>>>>> --- a/northd/northd.c
>>>>>>> +++ b/northd/northd.c
>>>>>>> @@ -398,6 +398,22 @@ ovn_stage_to_datapath_type(enum ovn_stage
> stage)
>>>>>>> }
>>>>>>> }
>>>>>>>
>>>>>>> +static void
>>>>>>> +build_chassis_features(const struct northd_input *input_data,
>>>>>>> + struct chassis_features *chassis_features)
>>>>>>> +{
>>>>>>> + const struct sbrec_chassis *chassis;
>>>>>>> +
>>>>>>> + SBREC_CHASSIS_TABLE_FOR_EACH (chassis,
>>> input_data->sbrec_chassis) {
>>>>>>> + if (!smap_get_bool(&chassis->other_config,
>>> OVN_FEATURE_CT_LB_MARK,
>>>>>>> + false)) {
>>>>>>> + chassis_features->ct_lb_mark = false;
>>>>>>> + return;
>>>>>>> + }
>>>>>>> + }
>>>>>>> + chassis_features->ct_lb_mark = true;
>>>>>>> +}
>>>>>>> +
>>>>>>> struct ovn_chassis_qdisc_queues {
>>>>>>> struct hmap_node key_node;
>>>>>>> uint32_t queue_id;
>>>>>>> @@ -3803,12 +3819,13 @@ static bool
>>>>>>> build_lb_vip_actions(struct ovn_lb_vip *lb_vip,
>>>>>>> struct ovn_northd_lb_vip *lb_vip_nb,
>>>>>>> struct ds *action, char *selection_fields,
>>>>>>> - bool ls_dp)
>>>>>>> + bool ls_dp, bool ct_lb_mark)
>>>>>>> {
>>>>>>> + const char *ct_lb_action = ct_lb_mark ? "ct_lb_mark" : "ct_lb";
>>>>>>> bool skip_hash_fields = false, reject = false;
>>>>>>>
>>>>>>> if (lb_vip_nb->lb_health_check) {
>>>>>>> - ds_put_cstr(action, "ct_lb_mark(backends=");
>>>>>>> + ds_put_format(action, "%s(backends=", ct_lb_action);
>>>>>>>
>>>>>>> size_t n_active_backends = 0;
>>>>>>> for (size_t i = 0; i < lb_vip->n_backends; i++) {
>>>>>>> @@ -3841,7 +3858,7 @@ build_lb_vip_actions(struct ovn_lb_vip
> *lb_vip,
>>>>>>> } else if (lb_vip->empty_backend_rej && !lb_vip->n_backends)
> {
>>>>>>> reject = true;
>>>>>>> } else {
>>>>>>> - ds_put_format(action, "ct_lb_mark(backends=%s);",
>>>>>>> + ds_put_format(action, "%s(backends=%s);", ct_lb_action,
>>>>>>> lb_vip_nb->backend_ips);
>>>>>>> }
>>>>>>>
>>>>>>> @@ -5768,13 +5785,16 @@ build_pre_lb(struct ovn_datapath *od, const
>>> struct shash *meter_groups,
>>>>>>> }
>>>>>>>
>>>>>>> static void
>>>>>>> -build_pre_stateful(struct ovn_datapath *od, struct hmap *lflows)
>>>>>>> +build_pre_stateful(struct ovn_datapath *od,
>>>>>>> + const struct chassis_features *features,
>>>>>>> + struct hmap *lflows)
>>>>>>> {
>>>>>>> /* Ingress and Egress pre-stateful Table (Priority 0):
> Packets
>>> are
>>>>>>> * allowed by default. */
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 0, "1",
>>> "next;");
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 0, "1",
>>> "next;");
>>>>>>>
>>>>>>> + const char *ct_lb_action = features->ct_lb_mark ? "ct_lb_mark"
> :
>>> "ct_lb";
>>>>>>> const char *lb_protocols[] = {"tcp", "udp", "sctp"};
>>>>>>> struct ds actions = DS_EMPTY_INITIALIZER;
>>>>>>> struct ds match = DS_EMPTY_INITIALIZER;
>>>>>>> @@ -5785,8 +5805,8 @@ build_pre_stateful(struct ovn_datapath *od,
>>> struct hmap *lflows)
>>>>>>> ds_put_format(&match, REGBIT_CONNTRACK_NAT" == 1 && ip4
> &&
>>> %s",
>>>>>>> lb_protocols[i]);
>>>>>>> ds_put_format(&actions, REG_ORIG_DIP_IPV4 " = ip4.dst; "
>>>>>>> - REG_ORIG_TP_DPORT " = %s.dst;
>>> ct_lb_mark;",
>>>>>>> - lb_protocols[i]);
>>>>>>> + REG_ORIG_TP_DPORT " = %s.dst; %s;",
>>>>>>> + lb_protocols[i], ct_lb_action);
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 120,
>>>>>>> ds_cstr(&match), ds_cstr(&actions));
>>>>>>>
>>>>>>> @@ -5795,20 +5815,20 @@ build_pre_stateful(struct ovn_datapath *od,
>>> struct hmap *lflows)
>>>>>>> ds_put_format(&match, REGBIT_CONNTRACK_NAT" == 1 && ip6
> &&
>>> %s",
>>>>>>> lb_protocols[i]);
>>>>>>> ds_put_format(&actions, REG_ORIG_DIP_IPV6 " = ip6.dst; "
>>>>>>> - REG_ORIG_TP_DPORT " = %s.dst;
>>> ct_lb_mark;",
>>>>>>> - lb_protocols[i]);
>>>>>>> + REG_ORIG_TP_DPORT " = %s.dst; %s;",
>>>>>>> + lb_protocols[i], ct_lb_action);
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 120,
>>>>>>> ds_cstr(&match), ds_cstr(&actions));
>>>>>>> }
>>>>>>>
>>>>>>> - ds_destroy(&actions);
>>>>>>> - ds_destroy(&match);
>>>>>>> + ds_clear(&actions);
>>>>>>> + ds_put_format(&actions, "%s;", ct_lb_action);
>>>>>>>
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 110,
>>>>>>> - REGBIT_CONNTRACK_NAT" == 1", "ct_lb_mark;");
>>>>>>> + REGBIT_CONNTRACK_NAT" == 1", ds_cstr(&actions));
>>>>>>>
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 110,
>>>>>>> - REGBIT_CONNTRACK_NAT" == 1", "ct_lb_mark;");
>>>>>>> + REGBIT_CONNTRACK_NAT" == 1", ds_cstr(&actions));
>>>>>>>
>>>>>>> /* If REGBIT_CONNTRACK_DEFRAG is set as 1, then the packets
>>> should be
>>>>>>> * sent to conntrack for tracking and defragmentation. */
>>>>>>> @@ -5817,6 +5837,9 @@ build_pre_stateful(struct ovn_datapath *od,
>>> struct hmap *lflows)
>>>>>>>
>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_STATEFUL, 100,
>>>>>>> REGBIT_CONNTRACK_DEFRAG" == 1", "ct_next;");
>>>>>>> +
>>>>>>> + ds_destroy(&actions);
>>>>>>> + ds_destroy(&match);
>>>>>>> }
>>>>>>>
>>>>>>> static void
>>>>>>> @@ -6684,7 +6707,7 @@ build_qos(struct ovn_datapath *od, struct hmap
>>> *lflows) {
>>>>>>> }
>>>>>>>
>>>>>>> static void
>>>>>>> -build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb,
>>>>>>> +build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, bool
>>> ct_lb_mark,
>>>>>>> struct ds *match, struct ds *action,
>>>>>>> const struct shash *meter_groups)
>>>>>>> {
>>>>>>> @@ -6734,7 +6757,8 @@ build_lb_rules(struct hmap *lflows, struct
>>> ovn_northd_lb *lb,
>>>>>>> /* New connections in Ingress table. */
>>>>>>> const char *meter = NULL;
>>>>>>> bool reject = build_lb_vip_actions(lb_vip, lb_vip_nb,
>>> action,
>>>>>>> - lb->selection_fields,
>>> true);
>>>>>>> + lb->selection_fields,
>>> true,
>>>>>>> + ct_lb_mark);
>>>>>>>
>>>>>>> ds_put_format(match, "ct.new && %s.dst == %s", ip_match,
>>>>>>> lb_vip->vip_str);
>>>>>>> @@ -7583,6 +7607,7 @@ build_lswitch_flows(const struct hmap
>>> *datapaths,
>>>>>>> static void
>>>>>>> build_lswitch_lflows_pre_acl_and_acl(struct ovn_datapath *od,
>>>>>>> const struct hmap
>>> *port_groups,
>>>>>>> + const struct chassis_features
>>> *features,
>>>>>>> struct hmap *lflows,
>>>>>>> const struct shash
>>> *meter_groups)
>>>>>>> {
>>>>>>> @@ -7591,7 +7616,7 @@ build_lswitch_lflows_pre_acl_and_acl(struct
>>> ovn_datapath *od,
>>>>>>>
>>>>>>> build_pre_acls(od, port_groups, lflows);
>>>>>>> build_pre_lb(od, meter_groups, lflows);
>>>>>>> - build_pre_stateful(od, lflows);
>>>>>>> + build_pre_stateful(od, features, lflows);
>>>>>>> build_acl_hints(od, lflows);
>>>>>>> build_acls(od, lflows, port_groups, meter_groups);
>>>>>>> build_qos(od, lflows);
>>>>>>> @@ -9672,8 +9697,10 @@ build_lrouter_nat_flows_for_lb(struct
>>> ovn_lb_vip *lb_vip,
>>>>>>> struct ovn_northd_lb_vip *vips_nb,
>>>>>>> struct hmap *lflows,
>>>>>>> struct ds *match, struct ds
> *action,
>>>>>>> - const struct shash *meter_groups)
>>>>>>> + const struct shash *meter_groups,
>>>>>>> + bool ct_lb_mark)
>>>>>>> {
>>>>>>> + const char *ct_natted = ct_lb_mark ? "ct_mark.natted" :
>>> "ct_label.natted";
>>>>>>> char *skip_snat_new_action = NULL;
>>>>>>> char *skip_snat_est_action = NULL;
>>>>>>> char *new_match;
>>>>>>> @@ -9683,7 +9710,8 @@ build_lrouter_nat_flows_for_lb(struct
>>> ovn_lb_vip *lb_vip,
>>>>>>> ds_clear(action);
>>>>>>>
>>>>>>> bool reject = build_lb_vip_actions(lb_vip, vips_nb, action,
>>>>>>> - lb->selection_fields,
> false);
>>>>>>> + lb->selection_fields, false,
>>>>>>> + ct_lb_mark);
>>>>>>>
>>>>>>> /* Higher priority rules are added for load-balancing in DNAT
>>>>>>> * table. For every match (on a VIP[:port]), we add two
> flows.
>>>>>>> @@ -9714,13 +9742,13 @@ build_lrouter_nat_flows_for_lb(struct
>>> ovn_lb_vip *lb_vip,
>>>>>>> REG_ORIG_TP_DPORT_ROUTER" == %d",
>>>>>>> ds_cstr(match), lb->proto,
>>> lb_vip->vip_port);
>>>>>>> est_match = xasprintf("ct.est && %s && %s && "
>>>>>>> - REG_ORIG_TP_DPORT_ROUTER" == %d && "
>>>>>>> - "ct_mark.natted == 1",
>>>>>>> - ds_cstr(match), lb->proto,
>>> lb_vip->vip_port);
>>>>>>> + REG_ORIG_TP_DPORT_ROUTER" == %d && %s
>>> == 1",
>>>>>>> + ds_cstr(match), lb->proto,
>>> lb_vip->vip_port,
>>>>>>> + ct_natted);
>>>>>>> } else {
>>>>>>> new_match = xasprintf("ct.new && %s", ds_cstr(match));
>>>>>>> - est_match = xasprintf("ct.est && %s && ct_mark.natted ==
> 1",
>>>>>>> - ds_cstr(match));
>>>>>>> + est_match = xasprintf("ct.est && %s && %s == 1",
>>>>>>> + ds_cstr(match), ct_natted);
>>>>>>> }
>>>>>>>
>>>>>>> const char *ip_match = NULL;
>>>>>>> @@ -9930,8 +9958,9 @@ build_lrouter_nat_flows_for_lb(struct
>>> ovn_lb_vip *lb_vip,
>>>>>>>
>>>>>>> static void
>>>>>>> build_lswitch_flows_for_lb(struct ovn_northd_lb *lb, struct hmap
>>> *lflows,
>>>>>>> - const struct shash *meter_groups, struct
>>> ds *match,
>>>>>>> - struct ds *action)
>>>>>>> + const struct shash *meter_groups,
>>>>>>> + const struct chassis_features *features,
>>>>>>> + struct ds *match, struct ds *action)
>>>>>>> {
>>>>>>> if (!lb->n_nb_ls) {
>>>>>>> return;
>>>>>>> @@ -9967,7 +9996,8 @@ build_lswitch_flows_for_lb(struct
> ovn_northd_lb
>>> *lb, struct hmap *lflows,
>>>>>>> * a higher priority rule for load balancing below also
>>> commits the
>>>>>>> * connection, so it is okay if we do not hit the above match
>>> on
>>>>>>> * REGBIT_CONNTRACK_COMMIT. */
>>>>>>> - build_lb_rules(lflows, lb, match, action, meter_groups);
>>>>>>> + build_lb_rules(lflows, lb, features->ct_lb_mark,
>>>>>>> + match, action, meter_groups);
>>>>>>> }
>>>>>>>
>>>>>>> /* If there are any load balancing rules, we should send the
>>> packet to
>>>>>>> @@ -10037,8 +10067,9 @@ build_lrouter_defrag_flows_for_lb(struct
>>> ovn_northd_lb *lb,
>>>>>>>
>>>>>>> static void
>>>>>>> build_lrouter_flows_for_lb(struct ovn_northd_lb *lb, struct hmap
>>> *lflows,
>>>>>>> - const struct shash *meter_groups, struct
>>> ds *match,
>>>>>>> - struct ds *action)
>>>>>>> + const struct shash *meter_groups,
>>>>>>> + const struct chassis_features *features,
>>>>>>> + struct ds *match, struct ds *action)
>>>>>>> {
>>>>>>> if (!lb->n_nb_lr) {
>>>>>>> return;
>>>>>>> @@ -10049,7 +10080,7 @@ build_lrouter_flows_for_lb(struct
>>> ovn_northd_lb *lb, struct hmap *lflows,
>>>>>>>
>>>>>>> build_lrouter_nat_flows_for_lb(lb_vip, lb,
> &lb->vips_nb[i],
>>>>>>> lflows, match, action,
>>>>>>> - meter_groups);
>>>>>>> + meter_groups,
>>> features->ct_lb_mark);
>>>>>>>
>>>>>>> if (!build_empty_lb_event_flow(lb_vip, lb->nlb, match,
>>> action)) {
>>>>>>> continue;
>>>>>>> @@ -13522,6 +13553,7 @@ struct lswitch_flow_build_info {
>>>>>>> const struct shash *meter_groups;
>>>>>>> const struct hmap *lbs;
>>>>>>> const struct hmap *bfd_connections;
>>>>>>> + const struct chassis_features *features;
>>>>>>> char *svc_check_match;
>>>>>>> struct ds match;
>>>>>>> struct ds actions;
>>>>>>> @@ -13540,7 +13572,9 @@
>>> build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,
>>>>>>> struct
>>> lswitch_flow_build_info *lsi)
>>>>>>> {
>>>>>>> /* Build Logical Switch Flows. */
>>>>>>> - build_lswitch_lflows_pre_acl_and_acl(od, lsi->port_groups,
>>> lsi->lflows,
>>>>>>> + build_lswitch_lflows_pre_acl_and_acl(od, lsi->port_groups,
>>>>>>> + lsi->features,
>>>>>>> + lsi->lflows,
>>>>>>> lsi->meter_groups);
>>>>>>>
>>>>>>> build_fwd_group_lflows(od, lsi->lflows);
>>>>>>> @@ -13680,10 +13714,12 @@ build_lflows_thread(void *arg)
>>>>>>> build_lrouter_defrag_flows_for_lb(lb,
>>> lsi->lflows,
>>>>>>>
> &lsi->match);
>>>>>>> build_lrouter_flows_for_lb(lb, lsi->lflows,
>>>>>>> - lsi->meter_groups,
>>> &lsi->match,
>>>>>>> - &lsi->actions);
>>>>>>> + lsi->meter_groups,
>>>>>>> + lsi->features,
>>>>>>> + &lsi->match,
>>> &lsi->actions);
>>>>>>> build_lswitch_flows_for_lb(lb, lsi->lflows,
>>>>>>> lsi->meter_groups,
>>>>>>> + lsi->features,
>>>>>>> &lsi->match,
>>> &lsi->actions);
>>>>>>> }
>>>>>>> }
>>>>>>> @@ -13749,7 +13785,8 @@ build_lswitch_and_lrouter_flows(const struct
>>> hmap *datapaths,
>>>>>>> struct hmap *igmp_groups,
>>>>>>> const struct shash *meter_groups,
>>>>>>> const struct hmap *lbs,
>>>>>>> - const struct hmap *bfd_connections)
>>>>>>> + const struct hmap *bfd_connections,
>>>>>>> + const struct chassis_features
>>> *features)
>>>>>>> {
>>>>>>>
>>>>>>> char *svc_check_match = xasprintf("eth.dst == %s",
>>> svc_monitor_mac);
>>>>>>> @@ -13786,6 +13823,7 @@ build_lswitch_and_lrouter_flows(const struct
>>> hmap *datapaths,
>>>>>>> lsiv[index].meter_groups = meter_groups;
>>>>>>> lsiv[index].lbs = lbs;
>>>>>>> lsiv[index].bfd_connections = bfd_connections;
>>>>>>> + lsiv[index].features = features;
>>>>>>> lsiv[index].svc_check_match = svc_check_match;
>>>>>>> lsiv[index].thread_lflow_counter = 0;
>>>>>>> ds_init(&lsiv[index].match);
>>>>>>> @@ -13824,6 +13862,7 @@ build_lswitch_and_lrouter_flows(const struct
>>> hmap *datapaths,
>>>>>>> .meter_groups = meter_groups,
>>>>>>> .lbs = lbs,
>>>>>>> .bfd_connections = bfd_connections,
>>>>>>> + .features = features,
>>>>>>> .svc_check_match = svc_check_match,
>>>>>>> .match = DS_EMPTY_INITIALIZER,
>>>>>>> .actions = DS_EMPTY_INITIALIZER,
>>>>>>> @@ -13849,9 +13888,9 @@ build_lswitch_and_lrouter_flows(const struct
>>> hmap *datapaths,
>>>>>>> &lsi.match);
>>>>>>> build_lrouter_defrag_flows_for_lb(lb, lsi.lflows,
>>> &lsi.match);
>>>>>>> build_lrouter_flows_for_lb(lb, lsi.lflows,
>>> lsi.meter_groups,
>>>>>>> - &lsi.match, &lsi.actions);
>>>>>>> + lsi.features, &lsi.match,
>>> &lsi.actions);
>>>>>>> build_lswitch_flows_for_lb(lb, lsi.lflows,
>>> lsi.meter_groups,
>>>>>>> - &lsi.match, &lsi.actions);
>>>>>>> + lsi.features, &lsi.match,
>>> &lsi.actions);
>>>>>>> }
>>>>>>> stopwatch_stop(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
>>>>>>> stopwatch_start(LFLOWS_IGMP_STOPWATCH_NAME, time_msec());
>>>>>>> @@ -13998,7 +14037,8 @@ void build_lflows(struct lflow_input
>>> *input_data,
>>>>>>> input_data->port_groups,
>>> &lflows,
>>>>>>> &mcast_groups, &igmp_groups,
>>>>>>> input_data->meter_groups,
>>> input_data->lbs,
>>>>>>> - input_data->bfd_connections);
>>>>>>> + input_data->bfd_connections,
>>>>>>> + input_data->features);
>>>>>>>
>>>>>>> if (parallelization_state == STATE_INIT_HASH_SIZES) {
>>>>>>> parallelization_state = STATE_USE_PARALLELIZATION;
>>>>>>> @@ -15129,6 +15169,7 @@ northd_init(struct northd_data *data)
>>>>>>> hmap_init(&data->lbs);
>>>>>>> hmap_init(&data->bfd_connections);
>>>>>>> ovs_list_init(&data->lr_list);
>>>>>>> + memset(&data->features, 0, sizeof data->features);
>>>>>>> data->ovn_internal_version_changed = false;
>>>>>>> }
>>>>>>>
>>>>>>> @@ -15262,6 +15303,7 @@ ovnnb_db_run(struct northd_input
> *input_data,
>>>>>>> "ignore_lsp_down", true);
>>>>>>> default_acl_drop = smap_get_bool(&nb->options,
>>> "default_acl_drop", false);
>>>>>>>
>>>>>>> + build_chassis_features(input_data, &data->features);
>>>>>>> build_datapaths(input_data, ovnsb_txn, &data->datapaths,
>>> &data->lr_list);
>>>>>>> build_lbs(input_data, &data->datapaths, &data->lbs);
>>>>>>> build_ports(input_data, ovnsb_txn, sbrec_chassis_by_name,
>>>>>>> diff --git a/northd/northd.h b/northd/northd.h
>>>>>>> index fe8dad03a..07fcbcacc 100644
>>>>>>> --- a/northd/northd.h
>>>>>>> +++ b/northd/northd.h
>>>>>>> @@ -58,6 +58,10 @@ struct northd_input {
>>>>>>> struct ovsdb_idl_index *sbrec_static_mac_binding_by_lport_ip;
>>>>>>> };
>>>>>>>
>>>>>>> +struct chassis_features {
>>>>>>> + bool ct_lb_mark;
>>>>>>> +};
>>>>>>> +
>>>>>>> struct northd_data {
>>>>>>> /* Global state for 'en-northd'. */
>>>>>>> struct hmap datapaths;
>>>>>>> @@ -68,6 +72,7 @@ struct northd_data {
>>>>>>> struct hmap bfd_connections;
>>>>>>> struct ovs_list lr_list;
>>>>>>> bool ovn_internal_version_changed;
>>>>>>> + struct chassis_features features;
>>>>>>> };
>>>>>>>
>>>>>>> struct lflow_input {
>>>>>>> @@ -89,6 +94,7 @@ struct lflow_input {
>>>>>>> const struct shash *meter_groups;
>>>>>>> const struct hmap *lbs;
>>>>>>> const struct hmap *bfd_connections;
>>>>>>> + const struct chassis_features *features;
>>>>>>> bool ovn_internal_version_changed;
>>>>>>> };
>>>>>>>
>>>>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>>>>>> index 5bd0935e7..7bb6d33ac 100644
>>>>>>> --- a/tests/ovn-northd.at
>>>>>>> +++ b/tests/ovn-northd.at
>>>>>>> @@ -4957,7 +4957,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed
>>> 's/table=./table=?/' | sort], [0], [
>>>>>>> table=? (lr_out_snat ), priority=120 , match=(nd_ns),
>>> action=(next;)
>>>>>>> ])
>>>>>>>
>>>>>>> -ovn-sbctl chassis-add gw1 geneve 127.0.0.1
>>>>>>> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \
>>>>>>> + -- set chassis gw1 other_config:ct-lb-mark="true"
>>>>>>>
>>>>>>> # Create a distributed gw port on lr0
>>>>>>> check ovn-nbctl ls-add public
>>>>>>> @@ -7418,3 +7419,69 @@ AT_CHECK([cat sw0flows | grep -e port_sec |
>>> sort | sed 's/table=./table=?/' ], [
>>>>>>>
>>>>>>> AT_CLEANUP
>>>>>>> ])
>>>>>>> +
>>>>>>> +
>>>>>>> +OVN_FOR_EACH_NORTHD([
>>>>>>> +AT_SETUP([Load balancer ct_lb_mark backwards compatibility])
>>>>>>> +AT_KEYWORDS([lb])
>>>>>>> +ovn_start
>>>>>>> +
>>>>>>> +check ovn-nbctl \
>>>>>>> + -- ls-add ls \
>>>>>>> + -- lr-add lr -- set logical_router lr options:chassis=local \
>>>>>>> + -- lb-add lb-test 66.66.66.66 42.42.42.2 \
>>>>>>> + -- ls-lb-add ls lb-test \
>>>>>>> + -- lr-lb-add lr lb-test
>>>>>>> +
>>>>>>> +AS_BOX([No chassis registered - use ct_lb_mark and ct_mark.natted])
>>>>>>> +check ovn-nbctl --wait=sb sync
>>>>>>> +AT_CHECK([ovn-sbctl lflow-list | grep -e natted -e ct_lb], [0],
> [dnl
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.est &&
>>> ip4 && reg0 == 66.66.66.66 && ct_mark.natted == 1), action=(next;)
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.new &&
>>> ip4 && reg0 == 66.66.66.66), action=(ct_lb_mark(backends=42.42.42.2);)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && sctp), action=(reg1 = ip4.dst; reg2[[0..15]] = sctp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && tcp), action=(reg1 = ip4.dst; reg2[[0..15]] = tcp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && udp), action=(reg1 = ip4.dst; reg2[[0..15]] = udp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && sctp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = sctp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && tcp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = tcp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && udp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = udp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb_mark;)
>>>>>>> + table=11(ls_in_lb ), priority=110 , match=(ct.new &&
>>> ip4.dst == 66.66.66.66), action=(reg0[[1]] = 0; reg1 = 66.66.66.66;
>>> ct_lb_mark(backends=42.42.42.2);)
>>>>>>> + table=2 (ls_out_pre_stateful), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb_mark;)
>>>>>>> +])
>>>>>>> +
>>>>>>> +AS_BOX([Chassis registered that doesn't support ct_lb_mark - use
>>> ct_lb and ct_label.natted])
>>>>>>> +check ovn-sbctl chassis-add hv geneve 127.0.0.1
>>>>>>> +check ovn-nbctl --wait=sb sync
>>>>>>> +AT_CHECK([ovn-sbctl lflow-list | grep -e natted -e ct_lb], [0],
> [dnl
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.est &&
>>> ip4 && reg0 == 66.66.66.66 && ct_label.natted == 1), action=(next;)
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.new &&
>>> ip4 && reg0 == 66.66.66.66), action=(ct_lb(backends=42.42.42.2);)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && sctp), action=(reg1 = ip4.dst; reg2[[0..15]] = sctp.dst;
> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && tcp), action=(reg1 = ip4.dst; reg2[[0..15]] = tcp.dst;
> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && udp), action=(reg1 = ip4.dst; reg2[[0..15]] = udp.dst;
> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && sctp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = sctp.dst;
>>> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && tcp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = tcp.dst;
> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && udp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = udp.dst;
> ct_lb;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb;)
>>>>>>> + table=11(ls_in_lb ), priority=110 , match=(ct.new &&
>>> ip4.dst == 66.66.66.66), action=(reg0[[1]] = 0; reg1 = 66.66.66.66;
>>> ct_lb(backends=42.42.42.2);)
>>>>>>> + table=2 (ls_out_pre_stateful), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb;)
>>>>>>> +])
>>>>>>> +
>>>>>>> +AS_BOX([Chassis upgrades and supports ct_lb_mark - use ct_lb_mark
>>> and ct_mark.natted])
>>>>>>> +check ovn-sbctl set chassis hv other_config:ct-lb-mark=true
>>>>>>> +check ovn-nbctl --wait=sb sync
>>>>>>> +AT_CHECK([ovn-sbctl lflow-list | grep -e natted -e ct_lb], [0],
> [dnl
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.est &&
>>> ip4 && reg0 == 66.66.66.66 && ct_mark.natted == 1), action=(next;)
>>>>>>> + table=6 (lr_in_dnat ), priority=110 , match=(ct.new &&
>>> ip4 && reg0 == 66.66.66.66), action=(ct_lb_mark(backends=42.42.42.2);)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && sctp), action=(reg1 = ip4.dst; reg2[[0..15]] = sctp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && tcp), action=(reg1 = ip4.dst; reg2[[0..15]] = tcp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip4 && udp), action=(reg1 = ip4.dst; reg2[[0..15]] = udp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && sctp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = sctp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && tcp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = tcp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=120 , match=(reg0[[2]]
> ==
>>> 1 && ip6 && udp), action=(xxreg1 = ip6.dst; reg2[[0..15]] = udp.dst;
>>> ct_lb_mark;)
>>>>>>> + table=6 (ls_in_pre_stateful ), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb_mark;)
>>>>>>> + table=11(ls_in_lb ), priority=110 , match=(ct.new &&
>>> ip4.dst == 66.66.66.66), action=(reg0[[1]] = 0; reg1 = 66.66.66.66;
>>> ct_lb_mark(backends=42.42.42.2);)
>>>>>>> + table=2 (ls_out_pre_stateful), priority=110 , match=(reg0[[2]]
> ==
>>> 1), action=(ct_lb_mark;)
>>>>>>> +])
>>>>>>> +
>>>>>>> +AT_CLEANUP
>>>>>>> +])
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> dev mailing list
>>>>>> [email protected]
>>>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> dev mailing list
>>>> [email protected]
>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>>
>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
