On 1/18/23 16:24, Numan Siddique wrote: > On Wed, Jan 18, 2023 at 7:59 AM Dumitru Ceara <[email protected]> wrote: >> >> On 1/18/23 13:27, Numan Siddique wrote: >>> On Thu, Jan 12, 2023 at 6:26 AM Dumitru Ceara <[email protected]> wrote: >>>> >>>> On 1/12/23 11:35, Ales Musil wrote: >>>>> On Thu, Jan 12, 2023 at 11:30 AM Dumitru Ceara <[email protected]> wrote: >>>>> >>>>>> On 1/10/23 16:58, Ales Musil wrote: >>>>>>> On Mon, Jan 9, 2023 at 3:35 PM Ales Musil <[email protected]> wrote: >>>>>>> >>>>>>>> In order to be backward compatible add feature flag >>>>>>>> that ensures that the CT related flows are skipped >>>>>>>> if needed. >>>>>>>> >>>>>>>> Signed-off-by: Ales Musil <[email protected]> >>>>>>>> --- >>>>>>>> v2: Fix the wrong * position. >>>>>>>> --- >>>>>>>> controller/chassis.c | 7 +++ >>>>>>>> include/ovn/features.h | 1 + >>>>>>>> northd/northd.c | 51 +++++++++++++------ >>>>>>>> northd/northd.h | 1 + >>>>>>>> tests/ovn-northd.at | 109 +++++++++++++++++++++++++++++++++++++++-- >>>>>>>> 5 files changed, 149 insertions(+), 20 deletions(-) >>>>>>>> >>>>>>>> diff --git a/controller/chassis.c b/controller/chassis.c >>>>>>>> index 685d9b2ae..98f8da2be 100644 >>>>>>>> --- a/controller/chassis.c >>>>>>>> +++ b/controller/chassis.c >>>>>>>> @@ -352,6 +352,7 @@ chassis_build_other_config(const struct >>>>>>>> ovs_chassis_cfg *ovs_cfg, >>>>>>>> smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true"); >>>>>>>> smap_replace(config, OVN_FEATURE_CT_NO_MASKED_LABEL, "true"); >>>>>>>> smap_replace(config, OVN_FEATURE_MAC_BINDING_TIMESTAMP, "true"); >>>>>>>> + smap_replace(config, OVN_FEATURE_CT_LB_RELATED, "true"); >>>>>>>> } >>>>>>>> >>>>>>>> /* >>>>>>>> @@ -469,6 +470,12 @@ 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_RELATED, >>>>>>>> + false)) { >>>>>>>> + return true; >>>>>>>> + } >>>>>>>> + >>>>>>>> return false; >>>>>>>> } >>>>>>>> >>>>>>>> diff --git a/include/ovn/features.h b/include/ovn/features.h >>>>>>>> index 679f67457..5bcd68739 100644 >>>>>>>> --- a/include/ovn/features.h >>>>>>>> +++ b/include/ovn/features.h >>>>>>>> @@ -24,6 +24,7 @@ >>>>>>>> #define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif" >>>>>>>> #define OVN_FEATURE_CT_NO_MASKED_LABEL "ct-no-masked-label" >>>>>>>> #define OVN_FEATURE_MAC_BINDING_TIMESTAMP "mac-binding-timestamp" >>>>>>>> +#define OVN_FEATURE_CT_LB_RELATED "ovn-ct-lb-related" >>>>>>>> >>>>>>>> /* OVS datapath supported features. Based on availability OVN might >>>>>>>> generate >>>>>>>> * different types of openflows. >>>>>>>> diff --git a/northd/northd.c b/northd/northd.c >>>>>>>> index 4751feab4..72f7118a8 100644 >>>>>>>> --- a/northd/northd.c >>>>>>>> +++ b/northd/northd.c >>>>>>>> @@ -447,6 +447,15 @@ build_chassis_features(const struct northd_input >>>>>>>> *input_data, >>>>>>>> chassis_features->mac_binding_timestamp) { >>>>>>>> chassis_features->mac_binding_timestamp = false; >>>>>>>> } >>>>>>>> + >>>>>>>> + bool ct_lb_related = >>>>>>>> + smap_get_bool(&chassis->other_config, >>>>>>>> + OVN_FEATURE_CT_LB_RELATED, >>>>>>>> + false); >>>>>>>> + if (!ct_lb_related && >>>>>>>> + chassis_features->ct_lb_related) { >>>>>>>> + chassis_features->ct_lb_related = false; >>>>>>>> + } >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> @@ -6786,14 +6795,17 @@ build_acls(struct ovn_datapath *od, const >>>>>>>> struct >>>>>>>> chassis_features *features, >>>>>>>> * a dynamically negotiated FTP data channel), but will allow >>>>>>>> * related traffic such as an ICMP Port Unreachable through >>>>>>>> * that's generated from a non-listening UDP port. */ >>>>>>>> + const char *ct_acl_action = features->ct_lb_related >>>>>>>> + ? "ct_commit_nat;" >>>>>>>> + : "next;"; >>>>>>>> ds_clear(&match); >>>>>>>> ds_put_format(&match, "!ct.est && ct.rel && !ct.new%s && %s == >>>>>> 0", >>>>>>>> use_ct_inv_match ? " && !ct.inv" : "", >>>>>>>> ct_blocked_match); >>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3, >>>>>>>> - ds_cstr(&match), "ct_commit_nat;"); >>>>>>>> + ds_cstr(&match), ct_acl_action); >>>>>>>> ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, >>>>>>>> - ds_cstr(&match), "ct_commit_nat;"); >>>>>>>> + ds_cstr(&match), ct_acl_action); >>>>>>>> >>>>>>>> /* Ingress and Egress ACL Table (Priority 65532). >>>>>>>> * >>>>>>>> @@ -10484,9 +10496,11 @@ build_lrouter_nat_flows_for_lb(struct >>>>>> ovn_lb_vip >>>>>>>> *lb_vip, >>>>>>>> struct hmap *lflows, >>>>>>>> struct ds *match, struct ds *action, >>>>>>>> const struct shash *meter_groups, >>>>>>>> - bool ct_lb_mark) >>>>>>>> + const struct chassis_features >>>>>>>> *features) >>>>>>>> { >>>>>>>> - const char *ct_natted = ct_lb_mark ? "ct_mark.natted" : >>>>>>>> "ct_label.natted"; >>>>>>>> + const char *ct_natted = features->ct_no_masked_label >>>>>>>> + ? "ct_mark.natted" >>>>>>>> + : "ct_label.natted"; >>>>>>>> char *skip_snat_new_action = NULL; >>>>>>>> char *skip_snat_est_action = NULL; >>>>>>>> char *new_match; >>>>>>>> @@ -10497,7 +10511,7 @@ build_lrouter_nat_flows_for_lb(struct >>>>>>>> ovn_lb_vip >>>>>>>> *lb_vip, >>>>>>>> >>>>>>>> bool reject = build_lb_vip_actions(lb_vip, vips_nb, action, >>>>>>>> lb->selection_fields, false, >>>>>>>> - ct_lb_mark); >>>>>>>> + features->ct_no_masked_label); >>>>>>>> bool drop = !!strncmp(ds_cstr(action), "ct_lb", strlen("ct_lb")); >>>>>>>> if (!drop) { >>>>>>>> /* Remove the trailing ");". */ >>>>>>>> @@ -10519,9 +10533,11 @@ build_lrouter_nat_flows_for_lb(struct >>>>>> ovn_lb_vip >>>>>>>> *lb_vip, >>>>>>>> } >>>>>>>> >>>>>>>> if (lb->skip_snat) { >>>>>>>> + const char *skip_snat = features->ct_lb_related && !drop >>>>>>>> + ? "; skip_snat);" >>>>>>>> + : ""; >>>>>>>> skip_snat_new_action = xasprintf("flags.skip_snat_for_lb = 1; >>>>>>>> %s%s", >>>>>>>> - ds_cstr(action), >>>>>>>> - drop ? "" : "; skip_snat);"); >>>>>>>> + ds_cstr(action), skip_snat); >>>>>>>> skip_snat_est_action = xasprintf("flags.skip_snat_for_lb = 1; >>>>>>>> " >>>>>>>> "next;"); >>>>>>>> } >>>>>>>> @@ -10654,9 +10670,11 @@ build_lrouter_nat_flows_for_lb(struct >>>>>> ovn_lb_vip >>>>>>>> *lb_vip, >>>>>>>> skip_snat_new_action, est_match, >>>>>>>> skip_snat_est_action, lflows, prio, meter_groups); >>>>>>>> >>>>>>>> + const char *force_snat = features->ct_lb_related && !drop >>>>>>>> + ? "; force_snat);" >>>>>>>> + : ""; >>>>>>>> char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s%s", >>>>>>>> - ds_cstr(action), >>>>>>>> - drop ? "" : "; force_snat);"); >>>>>>>> + ds_cstr(action), force_snat); >>>>>>>> build_gw_lrouter_nat_flows_for_lb(lb, gw_router_force_snat, >>>>>>>> n_gw_router_force_snat, reject, new_match, >>>>>>>> new_actions, est_match, >>>>>>>> @@ -10911,7 +10929,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, >>>>>>>> - features->ct_no_masked_label); >>>>>>>> + features); >>>>>>>> >>>>>>>> if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) { >>>>>>>> continue; >>>>>>>> @@ -14221,7 +14239,7 @@ build_lrouter_nat_defrag_and_lb(struct >>>>>>>> ovn_datapath *od, struct hmap *lflows, >>>>>>>> const struct hmap *ports, struct ds >>>>>>>> *match, >>>>>>>> struct ds *actions, >>>>>>>> const struct shash *meter_groups, >>>>>>>> - bool ct_lb_mark) >>>>>>>> + const struct chassis_features >>>>>> *features) >>>>>>>> { >>>>>>>> if (!od->nbr) { >>>>>>>> return; >>>>>>>> @@ -14252,9 +14270,11 @@ build_lrouter_nat_defrag_and_lb(struct >>>>>>>> ovn_datapath *od, struct hmap *lflows, >>>>>>>> * a dynamically negotiated FTP data channel), but will allow >>>>>>>> * related traffic such as an ICMP Port Unreachable through >>>>>>>> * that's generated from a non-listening UDP port. */ >>>>>>>> - if (od->has_lb_vip) { >>>>>>>> + if (od->has_lb_vip && features->ct_lb_related) { >>>>>>>> ds_clear(match); >>>>>>>> - const char *ct_flag_reg = ct_lb_mark ? "ct_mark" : "ct_label"; >>>>>>>> + const char *ct_flag_reg = features->ct_no_masked_label >>>>>>>> + ? "ct_mark" >>>>>>>> + : "ct_label"; >>>>>>>> >>>>>>>> ds_put_cstr(match, "ct.rel && !ct.est && !ct.new"); >>>>>>>> size_t match_len = match->length; >>>>>>>> @@ -14477,7 +14497,7 @@ build_lrouter_nat_defrag_and_lb(struct >>>>>>>> ovn_datapath *od, struct hmap *lflows, >>>>>>>> >>>>>>>> if (od->nbr->n_nat) { >>>>>>>> ds_clear(match); >>>>>>>> - const char *ct_natted = ct_lb_mark ? >>>>>>>> + const char *ct_natted = features->ct_no_masked_label ? >>>>>>>> "ct_mark.natted" : >>>>>>>> "ct_label.natted"; >>>>>>>> ds_put_format(match, "ip && %s == 1", ct_natted); >>>>>>>> @@ -14594,7 +14614,7 @@ build_lswitch_and_lrouter_iterate_by_od(struct >>>>>>>> ovn_datapath *od, >>>>>>>> build_lrouter_arp_nd_for_datapath(od, lsi->lflows, >>>>>> lsi->meter_groups); >>>>>>>> build_lrouter_nat_defrag_and_lb(od, lsi->lflows, lsi->ports, >>>>>>>> &lsi->match, >>>>>>>> &lsi->actions, lsi->meter_groups, >>>>>>>> - >>>>>>>> lsi->features->ct_no_masked_label); >>>>>>>> + lsi->features); >>>>>>>> build_lb_affinity_default_flows(od, lsi->lflows); >>>>>>>> } >>>>>>>> >>>>>>>> @@ -16086,6 +16106,7 @@ northd_init(struct northd_data *data) >>>>>>>> data->features = (struct chassis_features) { >>>>>>>> .ct_no_masked_label = true, >>>>>>>> .mac_binding_timestamp = true, >>>>>>>> + .ct_lb_related = true, >>>>>>>> }; >>>>>>>> data->ovn_internal_version_changed = false; >>>>>>>> } >>>>>>>> diff --git a/northd/northd.h b/northd/northd.h >>>>>>>> index ff8727cb7..4d9055296 100644 >>>>>>>> --- a/northd/northd.h >>>>>>>> +++ b/northd/northd.h >>>>>>>> @@ -71,6 +71,7 @@ struct northd_input { >>>>>>>> struct chassis_features { >>>>>>>> bool ct_no_masked_label; >>>>>>>> bool mac_binding_timestamp; >>>>>>>> + bool ct_lb_related; >>>>>>>> }; >>>>>>>> >>>>>>>> struct northd_data { >>>>>>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>>>>>>> index 56c1e6c2e..2eb735bbe 100644 >>>>>>>> --- a/tests/ovn-northd.at >>>>>>>> +++ b/tests/ovn-northd.at >>>>>>>> @@ -5127,7 +5127,8 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sed >>>>>>>> 's/table=./table=?/' | sort], [0], [ >>>>>>>> ]) >>>>>>>> >>>>>>>> check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 \ >>>>>>>> - -- set chassis gw1 other_config:ct-no-masked-label="true" >>>>>>>> + -- set chassis gw1 other_config:ct-no-masked-label="true" \ >>>>>>>> + -- set chassis gw1 other_config:ovn-ct-lb-related="true" >>>>>>>> >>>>>>>> # Create a distributed gw port on lr0 >>>>>>>> check ovn-nbctl ls-add public >>>>>>>> @@ -7875,7 +7876,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep >>>>>>>> 'ls.*acl.*blocked' ], [0], [dnl >>>>>>>> table=7 (ls_in_acl_hint ), priority=4 , match=(!ct.new && >>>>>> ct.est >>>>>>>> && !ct.rpl && ct_label.blocked == 0), action=(reg0[[8]] = 1; reg0[[10]] >>>>>> = >>>>>>>> 1; next;) >>>>>>>> table=7 (ls_in_acl_hint ), priority=2 , match=(ct.est && >>>>>>>> ct_label.blocked == 1), action=(reg0[[9]] = 1; next;) >>>>>>>> table=7 (ls_in_acl_hint ), priority=1 , match=(ct.est && >>>>>>>> ct_label.blocked == 0), action=(reg0[[10]] = 1; next;) >>>>>>>> - table=8 (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), >>>>>>>> action=(ct_commit_nat;) >>>>>>>> + table=8 (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;) >>>>>>>> table=8 (ls_in_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), >>>>>>>> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;) >>>>>>>> table=8 (ls_in_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_label.blocked == 1)), action=(drop;) >>>>>>>> table=8 (ls_in_acl ), priority=1 , match=(ip && ct.est >>>>>>>> && >>>>>>>> ct_label.blocked == 1), action=(reg0[[1]] = 1; next;) >>>>>>>> @@ -7883,7 +7884,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep >>>>>>>> 'ls.*acl.*blocked' ], [0], [dnl >>>>>>>> table=3 (ls_out_acl_hint ), priority=4 , match=(!ct.new && >>>>>> ct.est >>>>>>>> && !ct.rpl && ct_label.blocked == 0), action=(reg0[[8]] = 1; reg0[[10]] >>>>>> = >>>>>>>> 1; next;) >>>>>>>> table=3 (ls_out_acl_hint ), priority=2 , match=(ct.est && >>>>>>>> ct_label.blocked == 1), action=(reg0[[9]] = 1; next;) >>>>>>>> table=3 (ls_out_acl_hint ), priority=1 , match=(ct.est && >>>>>>>> ct_label.blocked == 0), action=(reg0[[10]] = 1; next;) >>>>>>>> - table=4 (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), >>>>>>>> action=(ct_commit_nat;) >>>>>>>> + table=4 (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;) >>>>>>>> table=4 (ls_out_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), >>>>>> action=(next;) >>>>>>>> table=4 (ls_out_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_label.blocked == 1)), action=(drop;) >>>>>>>> table=4 (ls_out_acl ), priority=1 , match=(ip && ct.est >>>>>>>> && >>>>>>>> ct_label.blocked == 1), action=(reg0[[1]] = 1; next;) >>>>>>>> @@ -7897,7 +7898,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep >>>>>>>> 'ls.*acl.*blocked' ], [0], [dnl >>>>>>>> table=7 (ls_in_acl_hint ), priority=4 , match=(!ct.new && >>>>>> ct.est >>>>>>>> && !ct.rpl && ct_mark.blocked == 0), action=(reg0[[8]] = 1; reg0[[10]] >>>>>> = 1; >>>>>>>> next;) >>>>>>>> table=7 (ls_in_acl_hint ), priority=2 , match=(ct.est && >>>>>>>> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;) >>>>>>>> table=7 (ls_in_acl_hint ), priority=1 , match=(ct.est && >>>>>>>> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;) >>>>>>>> - table=8 (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=8 (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(next;) >>>>>>>> table=8 (ls_in_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>> action=(reg0[[9]] >>>>>>>> = 0; reg0[[10]] = 0; next;) >>>>>>>> table=8 (ls_in_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> table=8 (ls_in_acl ), priority=1 , match=(ip && ct.est >>>>>>>> && >>>>>>>> ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;) >>>>>>>> @@ -7905,7 +7906,7 @@ AT_CHECK([ovn-sbctl lflow-list | grep >>>>>>>> 'ls.*acl.*blocked' ], [0], [dnl >>>>>>>> table=3 (ls_out_acl_hint ), priority=4 , match=(!ct.new && >>>>>> ct.est >>>>>>>> && !ct.rpl && ct_mark.blocked == 0), action=(reg0[[8]] = 1; reg0[[10]] >>>>>> = 1; >>>>>>>> next;) >>>>>>>> table=3 (ls_out_acl_hint ), priority=2 , match=(ct.est && >>>>>>>> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;) >>>>>>>> table=3 (ls_out_acl_hint ), priority=1 , match=(ct.est && >>>>>>>> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;) >>>>>>>> - table=4 (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=4 (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(next;) >>>>>>>> table=4 (ls_out_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>>>> action=(next;) >>>>>>>> table=4 (ls_out_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> table=4 (ls_out_acl ), priority=1 , match=(ip && ct.est >>>>>>>> && >>>>>>>> ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;) >>>>>>>> @@ -8425,3 +8426,101 @@ check_row_count sb:Chassis_Template_Var 0 >>>>>>>> >>>>>>>> AT_CLEANUP >>>>>>>> ]) >>>>>>>> + >>>>>>>> +OVN_FOR_EACH_NORTHD_NO_HV([ >>>>>>>> +AT_SETUP([Load balancer CT related 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 192.168.0.1 192.168.1.10 \ >>>>>>>> + -- ls-lb-add ls lb-test \ >>>>>>>> + -- lr-lb-add lr lb-test >>>>>>>> + >>>>>>>> +m4_define([DUMP_FLOWS_SORTED], [sed 's/table=[[0-9]]\{1,2\}/table=?/' >>>>>>>> | >>>>>>>> sort]) >>>>>>>> + >>>>>>>> +AS_BOX([No chassis registered - CT related flows should be installed]) >>>>>>>> +check ovn-nbctl --wait=sb sync >>>>>>>> +ovn-sbctl dump-flows | DUMP_FLOWS_SORTED > lflows0 >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "lr_in_defrag" -e "lr_in_dnat" lflows0], [0], [dnl >>>>>>>> + table=? (lr_in_defrag ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_defrag ), priority=100 , match=(ip && ip4.dst >>>>>> == >>>>>>>> 192.168.0.1), action=(reg0 = 192.168.0.1; ct_dnat;) >>>>>>>> + table=? (lr_in_defrag ), priority=50 , match=(icmp || >>>>>>>> icmp6), >>>>>>>> action=(ct_dnat;) >>>>>>>> + table=? (lr_in_dnat ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.est && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1 && ct_mark.natted == 1), action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.new && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1), >>>>>> action=(ct_lb_mark(backends=192.168.1.10);) >>>>>>>> + table=? (lr_in_dnat ), priority=50 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new), action=(ct_commit_nat;) >>>>>>>> + table=? (lr_in_dnat ), priority=70 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb >>>>>> = >>>>>>>> 1; ct_commit_nat;) >>>>>>>> + table=? (lr_in_dnat ), priority=70 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = >>>>>> 1; >>>>>>>> ct_commit_nat;) >>>>>>>> +]) >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "ls_in_acl" -e "ls_out_acl" lflows0 | grep >>>>>>>> "priority=65532"], [0], [dnl >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>> action=(reg0[[9]] >>>>>>>> = 0; reg0[[10]] = 0; next;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>>>> action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> +]) >>>>>>>> + >>>>>>>> + >>>>>>>> +AS_BOX([Chassis registered that doesn't support CT related]) >>>>>>>> +check ovn-sbctl chassis-add hv geneve 127.0.0.1 >>>>>>>> +check ovn-nbctl --wait=sb sync >>>>>>>> +ovn-sbctl dump-flows | DUMP_FLOWS_SORTED > lflows1 >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "lr_in_defrag" -e "lr_in_dnat" lflows1], [0], [dnl >>>>>>>> + table=? (lr_in_defrag ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_defrag ), priority=100 , match=(ip && ip4.dst >>>>>> == >>>>>>>> 192.168.0.1), action=(reg0 = 192.168.0.1; ct_dnat;) >>>>>>>> + table=? (lr_in_dnat ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.est && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1 && ct_label.natted == 1), action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.new && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1), action=(ct_lb(backends=192.168.1.10);) >>>>>>>> +]) >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "ls_in_acl" -e "ls_out_acl" lflows1 | grep >>>>>>>> "priority=65532"], [0], [dnl >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), >>>>>>>> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_label.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_label.blocked == 0), action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0), >>>>>> action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_label.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> +]) >>>>>>>> + >>>>>>>> +AS_BOX([Chassis upgrades and supports CT related]) >>>>>>>> +check ovn-sbctl set chassis hv other_config:ct-no-masked-label=true >>>>>>>> +check ovn-sbctl set chassis hv other_config:ovn-ct-lb-related=true >>>>>>>> +check ovn-nbctl --wait=sb sync >>>>>>>> +ovn-sbctl dump-flows | DUMP_FLOWS_SORTED > lflows2 >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "lr_in_defrag" -e "lr_in_dnat" lflows2], [0], [dnl >>>>>>>> + table=? (lr_in_defrag ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_defrag ), priority=100 , match=(ip && ip4.dst >>>>>> == >>>>>>>> 192.168.0.1), action=(reg0 = 192.168.0.1; ct_dnat;) >>>>>>>> + table=? (lr_in_defrag ), priority=50 , match=(icmp || >>>>>>>> icmp6), >>>>>>>> action=(ct_dnat;) >>>>>>>> + table=? (lr_in_dnat ), priority=0 , match=(1), >>>>>> action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.est && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1 && ct_mark.natted == 1), action=(next;) >>>>>>>> + table=? (lr_in_dnat ), priority=110 , match=(ct.new && >>>>>> !ct.rel >>>>>>>> && ip4 && reg0 == 192.168.0.1), >>>>>> action=(ct_lb_mark(backends=192.168.1.10);) >>>>>>>> + table=? (lr_in_dnat ), priority=50 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new), action=(ct_commit_nat;) >>>>>>>> + table=? (lr_in_dnat ), priority=70 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new && ct_mark.force_snat == 1), action=(flags.force_snat_for_lb >>>>>> = >>>>>>>> 1; ct_commit_nat;) >>>>>>>> + table=? (lr_in_dnat ), priority=70 , match=(ct.rel && >>>>>> !ct.est >>>>>>>> && !ct.new && ct_mark.skip_snat == 1), action=(flags.skip_snat_for_lb = >>>>>> 1; >>>>>>>> ct_commit_nat;) >>>>>>>> +]) >>>>>>>> + >>>>>>>> +AT_CHECK([grep -e "ls_in_acl" -e "ls_out_acl" lflows2 | grep >>>>>>>> "priority=65532"], [0], [dnl >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>> action=(reg0[[9]] >>>>>>>> = 0; reg0[[10]] = 0; next;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_in_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(!ct.est && >>>>>> ct.rel >>>>>>>> && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(ct_commit_nat;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.est && >>>>>> !ct.rel >>>>>>>> && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), >>>>>>>> action=(next;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(ct.inv || >>>>>> (ct.est >>>>>>>> && ct.rpl && ct_mark.blocked == 1)), action=(drop;) >>>>>>>> + table=? (ls_out_acl ), priority=65532, match=(nd || nd_ra || >>>>>>>> nd_rs || mldv1 || mldv2), action=(next;) >>>>>>>> +]) >>>>>>>> + >>>>>>>> +AT_CLEANUP >>>>>>>> +]) >>>>>>>> -- >>>>>>>> 2.39.0 >>>>>>>> >>>>>>>> >>>>>>> Just to make it clear, we want to backport the "ICMP related" series [0] >>>>>>> down to 21.12. This is needed so we are backward compatible with older >>>>>>> controllers on those branches. >>>>>>> Also that being said, we are missing Reported-at: tag for this patch. >>>>>>> >>>>>> >>>>>> Thanks for the explanation, Ales! The code looks OK to me but I have >>>>>> one complaint: the feature we're actually guarding is "ct_commit_nat". >>>>>> It happens to be used for LB related traffic. >>>>>> >>>>>> I think I'd change the feature name to OVN_FEATURE_CT_COMMIT_NAT or >>>>>> similar. >>>>>> >>>>>> What do you think? >>>>>> >>>>>> Thanks, >>>>>> Dumitru >>>>>> >>>>>> >>>>> It's a bit tricky. We are actually guarding both ct_commit_nat, but also >>>>> the support of skip_snat, >>>>> force_snat in ct_lb/ct_lb_mark commands. So that's the reason why it's not >>>>> called ct_commit_nat. >>>>> >>>> >>>> Ah, you're right. That's a bit unfortunate, I guess. Hoping that we >>>> don't have to backport more features in the future: >>>> >>>> Acked-by: Dumitru Ceara <[email protected]> >>> >>> Acked-by: Numan Siddique <[email protected]> >>> >>> One question before applying - Does this patch require updating the >>> documentation in ovn-northd.8.xml ? >>> >>> I'm not sure what we are doing for existing feature flags. >>> >> >> Until now we only documented the lflows that get generated if the >> feature is supported. I'm afraid it's too much noise if we document the >> backwards compatible version too. What do you think? > > I agree. I'm fine with the patch being merged. >
I applied this patch to the main branch and backported it to branch-22.12. Regards, Dumitru _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
