On Thu, Jun 2, 2022 at 8:22 AM Dumitru Ceara <[email protected]> wrote: > > Commit a075230e4a0f ("Use ct_mark for masked access to make flows > HW-offloading friendly.") started using the ct_mark.blocked and > ct_mark.ecmp_reply_port ct_label. 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_mark.blocked and ecmp_reply_port are 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]> > --- > controller/chassis.c | 6 ++ > include/ovn/features.h | 5 +- > northd/northd.c | 156 ++++++++++++++++++++++++++++++++---------------- > northd/northd.h | 1 > tests/ovn-northd.at | 92 ++++++++++++++++++++++++++++ > 5 files changed, 206 insertions(+), 54 deletions(-) > > diff --git a/controller/chassis.c b/controller/chassis.c > index 239658461..656f4c331 100644 > --- a/controller/chassis.c > +++ b/controller/chassis.c > @@ -351,6 +351,7 @@ chassis_build_other_config(const struct ovs_chassis_cfg *ovs_cfg, > ovs_cfg->is_interconn ? "true" : "false"); > smap_replace(config, OVN_FEATURE_PORT_UP_NOTIF, "true"); > smap_replace(config, OVN_FEATURE_CT_LB_MARK, "true"); > + smap_replace(config, OVN_FEATURE_CT_MASKED_MARK, "true"); > } > > /* > @@ -461,6 +462,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_MASKED_MARK, > + false)) { > + return true; > + } > + > return false; > } > > diff --git a/include/ovn/features.h b/include/ovn/features.h > index 09f002287..9aad22eda 100644 > --- a/include/ovn/features.h > +++ b/include/ovn/features.h > @@ -21,8 +21,9 @@ > #include "smap.h" > > /* ovn-controller supported feature names. */ > -#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif" > -#define OVN_FEATURE_CT_LB_MARK "ct-lb-mark" > +#define OVN_FEATURE_PORT_UP_NOTIF "port-up-notif" > +#define OVN_FEATURE_CT_LB_MARK "ct-lb-mark" > +#define OVN_FEATURE_CT_MASKED_MARK "ct-masked-mark"
The change looks good to me, just a minor thing: There is another dependency related to the change that avoids masked access to ct_label: the push/pop actions support in ovn-controller. But I think there is no need to add another feature name for that, because the ct-lb-mark and ct-masked-mark would already cover that dependency, because they are changed together in the same minor version. So instead I'd just use a single feature name e.g. "use-ct-mark" (or "avoid-masked-ct-label") to identify all the 3 related changes (ct-lb-mark, ct-masked-mark and "support-push-pop"). Acked-by: Han Zhou <[email protected]> > > /* 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 6e2f2a880..09b19d300 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -404,14 +404,20 @@ build_chassis_features(const struct northd_input *input_data, > { > const struct sbrec_chassis *chassis; > > + chassis_features->ct_lb_mark = true; > + chassis_features->ct_masked_mark = true; > SBREC_CHASSIS_TABLE_FOR_EACH (chassis, input_data->sbrec_chassis) { > - if (!smap_get_bool(&chassis->other_config, OVN_FEATURE_CT_LB_MARK, > - false)) { > + if (chassis_features->ct_lb_mark > + && !smap_get_bool(&chassis->other_config, > + OVN_FEATURE_CT_LB_MARK, false)) { > chassis_features->ct_lb_mark = false; > - return; > + } > + if (chassis_features->ct_masked_mark > + && !smap_get_bool(&chassis->other_config, > + OVN_FEATURE_CT_MASKED_MARK, false)) { > + chassis_features->ct_masked_mark = false; > } > } > - chassis_features->ct_lb_mark = true; > } > > struct ovn_chassis_qdisc_queues { > @@ -5868,7 +5874,9 @@ build_pre_stateful(struct ovn_datapath *od, > } > > static void > -build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > +build_acl_hints(struct ovn_datapath *od, > + const struct chassis_features *features, > + struct hmap *lflows) > { > /* This stage builds hints for the IN/OUT_ACL stage. Based on various > * combinations of ct flags packets may hit only a subset of the logical > @@ -5890,6 +5898,7 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > > for (size_t i = 0; i < ARRAY_SIZE(stages); i++) { > enum ovn_stage stage = stages[i]; > + const char *match; > > /* In any case, advance to the next stage. */ > if (!od->has_acls && !od->has_lb_vip) { > @@ -5919,8 +5928,10 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > * REGBIT_ACL_HINT_ALLOW_NEW. > * - drop ACLs. > */ > - ovn_lflow_add(lflows, od, stage, 6, > - "!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1", > + match = features->ct_masked_mark > + ? "!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1" > + : "!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1"; > + ovn_lflow_add(lflows, od, stage, 6, match, > REGBIT_ACL_HINT_ALLOW_NEW " = 1; " > REGBIT_ACL_HINT_DROP " = 1; " > "next;"); > @@ -5939,8 +5950,10 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > * connection must be committed with ct_mark.blocked set so we set > * REGBIT_ACL_HINT_BLOCK. > */ > - ovn_lflow_add(lflows, od, stage, 4, > - "!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 0", > + match = features->ct_masked_mark > + ? "!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 0" > + : "!ct.new && ct.est && !ct.rpl && ct_label.blocked == 0"; > + ovn_lflow_add(lflows, od, stage, 4, match, > REGBIT_ACL_HINT_ALLOW " = 1; " > REGBIT_ACL_HINT_BLOCK " = 1; " > "next;"); > @@ -5951,7 +5964,10 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > ovn_lflow_add(lflows, od, stage, 3, "!ct.est", > REGBIT_ACL_HINT_DROP " = 1; " > "next;"); > - ovn_lflow_add(lflows, od, stage, 2, "ct.est && ct_mark.blocked == 1", > + match = features->ct_masked_mark > + ? "ct.est && ct_mark.blocked == 1" > + : "ct.est && ct_label.blocked == 1"; > + ovn_lflow_add(lflows, od, stage, 2, match, > REGBIT_ACL_HINT_DROP " = 1; " > "next;"); > > @@ -5959,7 +5975,10 @@ build_acl_hints(struct ovn_datapath *od, struct hmap *lflows) > * drop ACLs in which case the connection must be committed with > * ct_mark.blocked set. > */ > - ovn_lflow_add(lflows, od, stage, 1, "ct.est && ct_mark.blocked == 0", > + match = features->ct_masked_mark > + ? "ct.est && ct_mark.blocked == 0" > + : "ct.est && ct_label.blocked == 0"; > + ovn_lflow_add(lflows, od, stage, 1, match, > REGBIT_ACL_HINT_BLOCK " = 1; " > "next;"); > } > @@ -6085,10 +6104,13 @@ build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, > > static void > consider_acl(struct hmap *lflows, struct ovn_datapath *od, > - struct nbrec_acl *acl, bool has_stateful, > + struct nbrec_acl *acl, bool has_stateful, bool ct_masked_mark, > const struct shash *meter_groups, struct ds *match, > struct ds *actions) > { > + const char *ct_blocked_match = ct_masked_mark > + ? "ct_mark.blocked" > + : "ct_label.blocked"; > bool ingress = !strcmp(acl->direction, "from-lport") ? true :false; > enum ovn_stage stage; > > @@ -6203,10 +6225,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, > ds_clear(actions); > > ds_put_format(match, "ct.est && !ct.rel && !ct.new%s && " > - "ct.rpl && ct_mark.blocked == 0 && " > + "ct.rpl && %s == 0 && " > "ct_label.label == %" PRId64, > use_ct_inv_match ? " && !ct.inv" : "", > - acl->label); > + ct_blocked_match, acl->label); > build_acl_log(actions, acl, meter_groups); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, log_related_stage, > @@ -6216,10 +6238,10 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, > > ds_clear(match); > ds_put_format(match, "!ct.est && ct.rel && !ct.new%s && " > - "ct_mark.blocked == 0 && " > + "%s == 0 && " > "ct_label.label == %" PRId64, > use_ct_inv_match ? " && !ct.inv" : "", > - acl->label); > + ct_blocked_match, acl->label); > ovn_lflow_add_with_hint(lflows, od, log_related_stage, > UINT16_MAX - 2, > ds_cstr(match), ds_cstr(actions), > @@ -6265,7 +6287,8 @@ consider_acl(struct hmap *lflows, struct ovn_datapath *od, > ds_clear(match); > ds_clear(actions); > ds_put_cstr(match, REGBIT_ACL_HINT_BLOCK " == 1"); > - ds_put_cstr(actions, "ct_commit { ct_mark.blocked = 1; }; "); > + ds_put_format(actions, "ct_commit { %s = 1; }; ", > + ct_blocked_match); > if (!strcmp(acl->action, "reject")) { > build_reject_acl_rules(od, lflows, stage, acl, match, > actions, &acl->header_, meter_groups); > @@ -6433,11 +6456,15 @@ build_port_group_lswitches(struct northd_input *input_data, > } > > static void > -build_acls(struct ovn_datapath *od, struct hmap *lflows, > - const struct hmap *port_groups, const struct shash *meter_groups) > +build_acls(struct ovn_datapath *od, const struct chassis_features *features, > + struct hmap *lflows, const struct hmap *port_groups, > + const struct shash *meter_groups) > { > const char *default_acl_action = default_acl_drop ? "drop;" : "next;"; > bool has_stateful = od->has_stateful_acl || od->has_lb_vip; > + const char *ct_blocked_match = features->ct_masked_mark > + ? "ct_mark.blocked" > + : "ct_label.blocked"; > struct ds match = DS_EMPTY_INITIALIZER; > struct ds actions = DS_EMPTY_INITIALIZER; > > @@ -6492,12 +6519,14 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, > * which will be done by ct_commit() in the "stateful" stage. > * Subsequent packets will hit the flow at priority 0 that just > * uses "next;". */ > + ds_clear(&match); > + ds_put_format(&match, "ip && ct.est && %s == 1", ct_blocked_match); > ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, 1, > - "ip && ct.est && ct_mark.blocked == 1", > - REGBIT_CONNTRACK_COMMIT" = 1; next;"); > + ds_cstr(&match), > + REGBIT_CONNTRACK_COMMIT" = 1; next;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, 1, > - "ip && ct.est && ct_mark.blocked == 1", > - REGBIT_CONNTRACK_COMMIT" = 1; next;"); > + ds_cstr(&match), > + REGBIT_CONNTRACK_COMMIT" = 1; next;"); > > default_acl_action = default_acl_drop > ? "drop;" > @@ -6515,8 +6544,9 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, > * > * This is enforced at a higher priority than ACLs can be defined. */ > ds_clear(&match); > - ds_put_format(&match, "%s(ct.est && ct.rpl && ct_mark.blocked == 1)", > - use_ct_inv_match ? "ct.inv || " : ""); > + ds_put_format(&match, "%s(ct.est && ct.rpl && %s == 1)", > + use_ct_inv_match ? "ct.inv || " : "", > + ct_blocked_match); > ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3, > ds_cstr(&match), "drop;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, > @@ -6533,8 +6563,9 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, > * This is enforced at a higher priority than ACLs can be defined. */ > ds_clear(&match); > ds_put_format(&match, "ct.est && !ct.rel && !ct.new%s && " > - "ct.rpl && ct_mark.blocked == 0", > - use_ct_inv_match ? " && !ct.inv" : ""); > + "ct.rpl && %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), REGBIT_ACL_HINT_DROP" = 0; " > REGBIT_ACL_HINT_BLOCK" = 0; next;"); > @@ -6553,9 +6584,9 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, > * related traffic such as an ICMP Port Unreachable through > * that's generated from a non-listening UDP port. */ > ds_clear(&match); > - ds_put_format(&match, "!ct.est && ct.rel && !ct.new%s && " > - "ct_mark.blocked == 0", > - use_ct_inv_match ? " && !ct.inv" : ""); > + 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), "next;"); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3, > @@ -6573,14 +6604,15 @@ build_acls(struct ovn_datapath *od, struct hmap *lflows, > /* Ingress or Egress ACL Table (Various priorities). */ > for (size_t i = 0; i < od->nbs->n_acls; i++) { > struct nbrec_acl *acl = od->nbs->acls[i]; > - consider_acl(lflows, od, acl, has_stateful, meter_groups, &match, > - &actions); > + consider_acl(lflows, od, acl, has_stateful, features->ct_masked_mark, > + meter_groups, &match, &actions); > } > struct ovn_port_group *pg; > HMAP_FOR_EACH (pg, key_node, port_groups) { > if (ovn_port_group_ls_find(pg, &od->nbs->header_.uuid)) { > for (size_t i = 0; i < pg->nb_pg->n_acls; i++) { > consider_acl(lflows, od, pg->nb_pg->acls[i], has_stateful, > + features->ct_masked_mark, > meter_groups, &match, &actions); > } > } > @@ -6819,8 +6851,15 @@ build_lb_rules(struct hmap *lflows, struct ovn_northd_lb *lb, bool ct_lb_mark, > } > > static void > -build_stateful(struct ovn_datapath *od, struct hmap *lflows) > +build_stateful(struct ovn_datapath *od, > + const struct chassis_features *features, > + struct hmap *lflows) > { > + const char *ct_block_action = features->ct_masked_mark > + ? "ct_mark.blocked" > + : "ct_label.blocked"; > + struct ds actions = DS_EMPTY_INITIALIZER; > + > /* Ingress LB, Ingress and Egress stateful Table (Priority 0): Packets are > * allowed by default. */ > ovn_lflow_add(lflows, od, S_SWITCH_IN_LB, 0, "1", "next;"); > @@ -6833,29 +6872,33 @@ build_stateful(struct ovn_datapath *od, struct hmap *lflows) > * We always set ct_mark.blocked to 0 here as > * any packet that makes it this far is part of a connection we > * want to allow to continue. */ > + ds_put_format(&actions, "ct_commit { %s = 0; " > + "ct_label.label = " REG_LABEL "; }; next;", > + ct_block_action); > ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, > REGBIT_CONNTRACK_COMMIT" == 1 && " > REGBIT_ACL_LABEL" == 1", > - "ct_commit { ct_mark.blocked = 0; " > - "ct_label.label = " REG_LABEL "; }; next;"); > + ds_cstr(&actions)); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100, > REGBIT_CONNTRACK_COMMIT" == 1 && " > REGBIT_ACL_LABEL" == 1", > - "ct_commit { ct_mark.blocked = 0; " > - "ct_label.label = " REG_LABEL "; }; next;"); > + ds_cstr(&actions)); > > /* If REGBIT_CONNTRACK_COMMIT is set as 1, then the packets should be > * committed to conntrack. We always set ct_mark.blocked to 0 here as > * any packet that makes it this far is part of a connection we > * want to allow to continue. */ > + ds_clear(&actions); > + ds_put_format(&actions, "ct_commit { %s = 0; }; next;", ct_block_action); > ovn_lflow_add(lflows, od, S_SWITCH_IN_STATEFUL, 100, > REGBIT_CONNTRACK_COMMIT" == 1 && " > REGBIT_ACL_LABEL" == 0", > - "ct_commit { ct_mark.blocked = 0; }; next;"); > + ds_cstr(&actions)); > ovn_lflow_add(lflows, od, S_SWITCH_OUT_STATEFUL, 100, > REGBIT_CONNTRACK_COMMIT" == 1 && " > REGBIT_ACL_LABEL" == 0", > - "ct_commit { ct_mark.blocked = 0; }; next;"); > + ds_cstr(&actions)); > + ds_destroy(&actions); > } > > static void > @@ -7642,10 +7685,10 @@ 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, features, lflows); > - build_acl_hints(od, lflows); > - build_acls(od, lflows, port_groups, meter_groups); > + build_acl_hints(od, features, lflows); > + build_acls(od, features, lflows, port_groups, meter_groups); > build_qos(od, lflows); > - build_stateful(od, lflows); > + build_stateful(od, features, lflows); > build_lb_hairpin(od, lflows); > build_vtep_hairpin(od, lflows); > } > @@ -9343,6 +9386,7 @@ find_static_route_outport(struct ovn_datapath *od, const struct hmap *ports, > static void > add_ecmp_symmetric_reply_flows(struct hmap *lflows, > struct ovn_datapath *od, > + bool ct_masked_mark, > const char *port_ip, > struct ovn_port *out_port, > const struct parsed_route *route, > @@ -9353,6 +9397,9 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows, > struct ds actions = DS_EMPTY_INITIALIZER; > struct ds ecmp_reply = DS_EMPTY_INITIALIZER; > char *cidr = normalize_v46_prefix(&route->prefix, route->plen); > + const char *ct_ecmp_reply_port_match = ct_masked_mark > + ? "ct_mark.ecmp_reply_port" > + : "ct_label.ecmp_reply_port"; > > /* If symmetric ECMP replies are enabled, then packets that arrive over > * an ECMP route need to go through conntrack. > @@ -9381,8 +9428,8 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows, > ds_put_cstr(&match, " && (ct.new && !ct.est)"); > > ds_put_format(&actions, "ct_commit { ct_label.ecmp_reply_eth = eth.src;" > - " ct_mark.ecmp_reply_port = %" PRId64 ";}; next;", > - out_port->sb->tunnel_key); > + " %s = %" PRId64 ";}; next;", > + ct_ecmp_reply_port_match, out_port->sb->tunnel_key); > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_ECMP_STATEFUL, 100, > ds_cstr(&match), ds_cstr(&actions), > &st_route->header_); > @@ -9390,8 +9437,8 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows, > /* Bypass ECMP selection if we already have ct_label information > * for where to route the packet. > */ > - ds_put_format(&ecmp_reply, "ct.rpl && ct_mark.ecmp_reply_port == %" > - PRId64, out_port->sb->tunnel_key); > + ds_put_format(&ecmp_reply, "ct.rpl && %s == %"PRId64, > + ct_ecmp_reply_port_match, out_port->sb->tunnel_key); > ds_clear(&match); > ds_put_format(&match, "%s && %s", ds_cstr(&ecmp_reply), > ds_cstr(route_match)); > @@ -9433,7 +9480,8 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows, > > static void > build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, > - const struct hmap *ports, struct ecmp_groups_node *eg) > + bool ct_masked_mark, const struct hmap *ports, > + struct ecmp_groups_node *eg) > > { > bool is_ipv4 = IN6_IS_ADDR_V4MAPPED(&eg->prefix); > @@ -9487,7 +9535,8 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, > if (smap_get(&od->nbr->options, "chassis") && > route_->ecmp_symmetric_reply && sset_add(&visited_ports, > out_port->key)) { > - add_ecmp_symmetric_reply_flows(lflows, od, lrp_addr_s, out_port, > + add_ecmp_symmetric_reply_flows(lflows, od, ct_masked_mark, > + lrp_addr_s, out_port, > route_, &route_match); > } > ds_clear(&match); > @@ -11218,8 +11267,9 @@ build_ip_routing_flows_for_lrouter_port( > > static void > build_static_route_flows_for_lrouter( > - struct ovn_datapath *od, struct hmap *lflows, > - const struct hmap *ports, const struct hmap *bfd_connections) > + struct ovn_datapath *od, const struct chassis_features *features, > + struct hmap *lflows, const struct hmap *ports, > + const struct hmap *bfd_connections) > { > if (od->nbr) { > ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150, > @@ -11262,7 +11312,8 @@ build_static_route_flows_for_lrouter( > HMAP_FOR_EACH (group, hmap_node, &ecmp_groups) { > /* add a flow in IP_ROUTING, and one flow for each member in > * IP_ROUTING_ECMP. */ > - build_ecmp_route_flow(lflows, od, ports, group); > + build_ecmp_route_flow(lflows, od, features->ct_masked_mark, > + ports, group); > } > const struct unique_routes_node *ur; > HMAP_FOR_EACH (ur, hmap_node, &unique_routes) { > @@ -13618,7 +13669,8 @@ build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od, > &lsi->actions, lsi->meter_groups); > build_ND_RA_flows_for_lrouter(od, lsi->lflows); > build_ip_routing_pre_flows_for_lrouter(od, lsi->lflows); > - build_static_route_flows_for_lrouter(od, lsi->lflows, lsi->ports, > + build_static_route_flows_for_lrouter(od, lsi->features, > + lsi->lflows, lsi->ports, > lsi->bfd_connections); > build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match, > &lsi->actions); > diff --git a/northd/northd.h b/northd/northd.h > index 07fcbcacc..e5705eb70 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -60,6 +60,7 @@ struct northd_input { > > struct chassis_features { > bool ct_lb_mark; > + bool ct_masked_mark; > }; > > struct northd_data { > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 4bb815c7b..a9988b1e9 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -5725,6 +5725,19 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192. > table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 2), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;) > table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;) > ]) > + > +dnl The chassis was created with other_config:ct-masked-mark=false, the flows > +dnl should be using ct_label.ecmp_reply_port. > +AT_CHECK([grep -e "lr_in_arp_resolve.*ecmp" lr0flows | sed 's/table=../table=??/'], [0], [dnl > + table=??(lr_in_arp_resolve ), priority=200 , match=(ct.rpl && ct_label.ecmp_reply_port == 1), action=(push(xxreg1); xxreg1 = ct_label; eth.dst = xxreg1[[32..79]]; pop(xxreg1); next;) > +]) > + > +dnl Simulate an ovn-controller upgrade to a version that supports > +dnl ct-masked-mark. ovn-northd should start using ct_mark.ecmp_reply_port. > + > +check ovn-sbctl set chassis ch1 other_config:ct-masked-mark=true > +check ovn-nbctl --wait=sb sync > +ovn-sbctl dump-flows lr0 > lr0flows > AT_CHECK([grep -e "lr_in_arp_resolve.*ecmp" lr0flows | sed 's/table=../table=??/'], [0], [dnl > table=??(lr_in_arp_resolve ), priority=200 , match=(ct.rpl && ct_mark.ecmp_reply_port == 1), action=(push(xxreg1); xxreg1 = ct_label; eth.dst = xxreg1[[32..79]]; pop(xxreg1); next;) > ]) > @@ -7485,3 +7498,82 @@ AT_CHECK([ovn-sbctl lflow-list | grep -e natted -e ct_lb], [0], [dnl > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ACL ct_mark.blocked backwards compatibility]) > +AT_KEYWORDS([acl]) > +ovn_start > + > +check ovn-nbctl \ > + -- ls-add ls \ > + -- acl-add ls from-lport 1 1 allow-related \ > + -- --apply-after-lb acl-add ls from-lport 1 1 allow-related \ > + -- acl-add ls to-lport 1 1 allow-related > + > +AS_BOX([No chassis registered - use ct_mark.blocked]) > +check ovn-nbctl --wait=sb sync > +AT_CHECK([ovn-sbctl lflow-list | grep 'ls.*acl.*blocked' ], [0], [dnl > + table=7 (ls_in_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > + table=3 (ls_out_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > +]) > + > +AS_BOX([Chassis registered that doesn't support ct_mark.blocked - use ct_label.blocked]) > +check ovn-sbctl chassis-add hv geneve 127.0.0.1 > +check ovn-nbctl --wait=sb sync > +AT_CHECK([ovn-sbctl lflow-list | grep 'ls.*acl.*blocked' ], [0], [dnl > + table=7 (ls_in_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > + table=3 (ls_out_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_label.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > +]) > + > +AS_BOX([Chassis upgrades and supports ct_mark.blocked - use ct_mark.blocked]) > +check ovn-sbctl set chassis hv other_config:ct-masked-mark=true > +check ovn-nbctl --wait=sb sync > +AT_CHECK([ovn-sbctl lflow-list | grep 'ls.*acl.*blocked' ], [0], [dnl > + table=7 (ls_in_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > + table=3 (ls_out_acl_hint ), priority=6 , match=(!ct.new && ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;) > + 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=(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;) > +]) > + > +AT_CLEANUP > +]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
