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.
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.

Thanks,
Han
> >
> >
> >
> >>
> >> 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

Reply via email to