| Message ID | 20250515115928.35028-1-arukomoinikova@k2.cloud |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,v5] northd: Add option to make work lb with stateless ACL. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 5/15/25 1:59 PM, Alexandra Rukomoinikova wrote: > [0] Removed support for using load balancers in conjunction with stateless ACL. > This commit adds an option enable-stateless-acl-with-lb for logical switch to > make load balancers work. > > [0] - https://github.com/ovn-org/ovn/commit/a0f82efdd9dfd3ef2d9606c1890e353df1097a51 > > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > --- > v4 --> v5: 1) made this functionality optional on the logical switch. > 2) moved snat_ip to ovn_northd_lb structure from lb_vip bc > it's seems to be more correct. > 3) added ls_lb_with_stateless_mode hmapx in ovn_lb_datapaths > because the flows for the load balancer are added to the datapath group, > and I would like to separately add the flows needed for the balancers > to work with stateless acl only for datapath with enabled option. > 4) added doc and NEWS. > --- Hi Alexandra, Thanks for v5! > NEWS | 3 + > northd/lb.c | 18 +++++- > northd/lb.h | 5 ++ > northd/northd.c | 62 +++++++++++++++++++- > northd/northd.h | 4 ++ > northd/ovn-northd.8.xml | 8 +++ > ovn-nb.xml | 8 +++ > tests/ovn-northd.at | 123 ++++++++++++++++++++++++++++++++++++++++ > tests/system-ovn.at | 91 +++++++++++++++++++++++++++++ > 9 files changed, 318 insertions(+), 4 deletions(-) > > diff --git a/NEWS b/NEWS > index f82f25754..52c829e83 100644 > --- a/NEWS > +++ b/NEWS > @@ -20,6 +20,9 @@ Post v25.03.0 > - Introduce exclude-router-ips-from-garp in logical_switch_port column > so that the router port IPs are not advertised in the GARPs. > - Added support for port mirroring in OVN overlay. > + - Add a new logical switch option - enable-stateless-acl-lb with default > + value of false. This option should be set to true for logical switch Nit: s/switch/switches/ > + with stateless ACL to work with load balancer. > > OVN v25.03.0 - 07 Mar 2025 > -------------------------- > diff --git a/northd/lb.c b/northd/lb.c > index ffd21c5bc..b11896cf1 100644 > --- a/northd/lb.c > +++ b/northd/lb.c > @@ -257,6 +257,14 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb, > return NULL; > } > > +static bool > +validate_snap_ip_address(const char *snat_ip) > +{ > + ovs_be32 ip; > + > + return ip_parse(snat_ip, &ip); > +} > + > static void > ovn_northd_lb_init(struct ovn_northd_lb *lb, > const struct nbrec_load_balancer *nbrec_lb) > @@ -297,6 +305,12 @@ ovn_northd_lb_init(struct ovn_northd_lb *lb, > } > lb->affinity_timeout = affinity_timeout; > > + const char *snat_ip = smap_get(&nbrec_lb->options, "hairpin_snat_ip"); > + > + if (snat_ip && validate_snap_ip_address(snat_ip)) { > + lb->hairpin_snat_ip = xstrdup(snat_ip); > + } > + > sset_init(&lb->ips_v4); > sset_init(&lb->ips_v6); > struct smap_node *node; > @@ -413,6 +427,7 @@ ovn_northd_lb_cleanup(struct ovn_northd_lb *lb) > sset_destroy(&lb->ips_v4); > sset_destroy(&lb->ips_v6); > free(lb->selection_fields); > + free(lb->hairpin_snat_ip); > lb->selection_fields = NULL; > lb->health_checks = false; > } > @@ -566,7 +581,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths, > lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths); > lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths); > lb_dps->lflow_ref = lflow_ref_create(); > - > + hmapx_init(&lb_dps->ls_lb_with_stateless_mode); > return lb_dps; > } > > @@ -576,6 +591,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps) > bitmap_free(lb_dps->nb_lr_map); > bitmap_free(lb_dps->nb_ls_map); > lflow_ref_destroy(lb_dps->lflow_ref); > + hmapx_destroy(&lb_dps->ls_lb_with_stateless_mode); > free(lb_dps); > } > > diff --git a/northd/lb.h b/northd/lb.h > index aa6616af4..eb1942bd4 100644 > --- a/northd/lb.h > +++ b/northd/lb.h > @@ -18,6 +18,7 @@ > #define OVN_NORTHD_LB_H 1 > > #include "openvswitch/hmap.h" > +#include "hmapx.h" > #include "uuid.h" > > #include "lib/lb.h" > @@ -72,6 +73,8 @@ struct ovn_northd_lb { > > /* Indicates if the load balancer has health checks configured. */ > bool health_checks; > + > + char *hairpin_snat_ip; > }; > > /* ovn-northd specific backend information. */ > @@ -139,6 +142,8 @@ struct ovn_lb_datapaths { > size_t n_nb_lr; > unsigned long *nb_lr_map; > > + struct hmapx ls_lb_with_stateless_mode; > + > /* Reference of lflows generated for this load balancer. > * > * This data is initialized and destroyed by the en_northd node, but > diff --git a/northd/northd.c b/northd/northd.c > index 7b05147b4..85a1fcc58 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -503,6 +503,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, > od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > + od->lb_with_stateless_mode = false; > return od; > } > > @@ -942,6 +943,12 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, > if (smap_get_bool(&nbs->other_config, "ic-vxlan_mode", false)) { > vxlan_ic_mode = true; > } > + > + if (smap_get_bool(&nbs->other_config, > + "enable-stateless-acl-with-lb", false)) { Nit: indentation is a bit off. > + VLOG_WARN("set option to true"); This VLOG_WARN should be removed. > + od->lb_with_stateless_mode = true; > + } > } > > const struct nbrec_logical_router *nbr; > @@ -3965,6 +3972,9 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, > lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); > ovs_assert(lb_dps); > ovn_lb_datapaths_add_ls(lb_dps, 1, &od); > + if (od->lb_with_stateless_mode) { > + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); > + } > } > > for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { > @@ -5534,6 +5544,10 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > ovs_assert(lb_dps); > ovn_lb_datapaths_add_ls(lb_dps, 1, &od); > > + if (od->lb_with_stateless_mode) { > + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); > + } > + > /* Add the lb to the northd tracked data. */ > hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); > } > @@ -6215,7 +6229,7 @@ build_stateless_filter(const struct ovn_datapath *od, > action, > &acl->header_, > lflow_ref); > - } else { > + } else if (!od->lb_with_stateless_mode) { > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL, > acl->priority + OVN_ACL_PRI_OFFSET, > acl->match, > @@ -7780,8 +7794,14 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, > * > * 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 || " : ""); > + > + if (use_ct_inv_match && !od->lb_with_stateless_mode) { > + ds_put_cstr(&match, "ct.inv || (ct.est && ct.rpl && " > + "ct_mark.blocked == 1)"); > + } else { > + ds_put_cstr(&match, "(ct.est && ct.rpl && ct_mark.blocked == 1)"); > + } > + > ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX - 3, > ds_cstr(&match), REGBIT_ACL_VERDICT_DROP " = 1; next;", > lflow_ref); > @@ -8086,6 +8106,42 @@ build_lb_rules_pre_stateful(struct lflow_table *lflows, > lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths), > S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match), ds_cstr(action), > &lb->nlb->header_, lb_dps->lflow_ref); > + > + struct lflow_ref *lflow_ref = lb_dps->lflow_ref; > + struct hmapx_node *hmapx_node; > + struct ovn_datapath *od; > + HMAPX_FOR_EACH (hmapx_node, &lb_dps->ls_lb_with_stateless_mode) { > + od = hmapx_node->data; > + > + ds_clear(action); > + ds_clear(match); > + > + ds_put_format(match, "%s.dst == %s", ip_match, lb_vip->vip_str); > + > + if (lb_vip->port_str) { > + ds_put_format(match, " && %s.dst == %s", lb->proto, > + lb_vip->port_str); Nit: indentation is a bit off. > + } > + > + ds_put_cstr(action, "ct_lb_mark;"); > + > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 150, > + ds_cstr(match), ds_cstr(action), lflow_ref); > + > + if (lb->hairpin_snat_ip || lb_vip->port_str) { > + ds_clear(action); > + ds_clear(match); > + > + ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match, > + lb->hairpin_snat_ip ? lb->hairpin_snat_ip > + : lb_vip->vip_str); I think this matches our style better: ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match, lb->hairpin_snat_ip ? lb->hairpin_snat_ip : lb_vip->vip_str); > + > + ds_put_cstr(action, "ct_lb_mark;"); > + > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 105, > + ds_cstr(match), ds_cstr(action), lflow_ref); > + } > + } > } > } > > diff --git a/northd/northd.h b/northd/northd.h > index 5a698458f..f32ff1e2e 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -379,6 +379,10 @@ struct ovn_datapath { > bool has_vtep_lports; > bool has_arp_proxy_port; > > + /* Set to true if the option 'enable-stateless-acl-with-lb' is enabled > + * on the logical switch. */ > + bool lb_with_stateless_mode; > + > /* IPAM data. */ > struct ipam_info ipam_info; > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 96e102eeb..5d00cff89 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -652,6 +652,12 @@ > <code>ct_lb_mark;</code> action. > </li> > > + <li> > + A priority-105 added enabled when enable-stateless-acl-with-lb and > + send all packet directed to VIP that don't match the above flows to > + connection tracker. > + </li> > + > <li> > A priority-100 flow sends the packets to connection tracker based > on a hint provided by the previous tables > @@ -2331,6 +2337,8 @@ output; > <p> > This is similar to ingress table <code>Pre-ACLs</code> except for > <code>to-lport</code> traffic. > + Except when the option enable-stateless-acl-with-lbis enabled: Typo: enable-stateless-acl-with-lbis enabled > + REGBIT_ACL_STATELESS ignored. > </p> > > <p> > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 4396023a4..94ee92b9c 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -839,6 +839,14 @@ > is not set and the zone limit is derived from OvS default datapath > limit. > </column> > + > + <column name="other_config" key="enable-stateless-acl-with-lb" > + type='{"type": "boolean"}'> > + This option must be set to true for stateless ACL to work with load > + balancers. When enabled, packets with the ct.inv flag will not be > + dropped, even if use_ct_inv_match is set to true. > + Default: <code>false</code>. > + </column> > </group> > > <group title="IP Multicast Snooping Options"> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 69b75fe9d..0eca60fdb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -17290,3 +17290,126 @@ AT_CHECK([cat trace | grep output], [0], [dnl > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([enable-stateless-acl-with-lb usage]) > +ovn_start ovn-northd > + > +AS_BOX([Create logical switches and ports.]) > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \ > +"00:00:00:00:00:02 10.0.0.2" > + > +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 \ > +"00:00:00:00:00:03 10.0.0.3" > + > +AS_BOX([Create stateless ACLs.]) > +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1001 "ip" allow-stateless > +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1001 "ip" allow-stateless > + > +AS_BOX([Create stateful ACLs.]) > +# check if allow-stateless acls have higher priority we skip conntrack. > +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1000 "ip" allow-related > +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1000 "ip" allow-related > + > +AS_BOX([Create Load Balancer.]) > +check ovn-nbctl lb-add lb1 10.0.0.4:80 10.0.0.2:80,10.0.0.3:80 > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 > + > +ovn-sbctl dump-flows sw0 > sw0flows > + > +AT_CHECK( > + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > +]) > + > +AT_CHECK( > + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl > + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) This part needs a small rebase since: 8828b6120ea1 ("northd: Remove redundant ct state matches.") https://github.com/ovn-org/ovn/commit/8828b61 > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.inv || (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > +]) > + > +AS_BOX([Enable enable-stateless-acl-with-lb option.]) > +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:enable-stateless-acl-with-lb=true > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK( > + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > +]) > + > +# We do not match conntrack invalide packets in case of load balancers with stateless ACLs. Typo: invalide > +AT_CHECK( > + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl > + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) > + table=??(ls_out_acl_eval ), priority=65532, match=((ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > +]) > + > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.4), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(reg4 = 10.0.0.4; reg2[[0..15]] = 80; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Create Load Balancer without port.]) > +check ovn-nbctl --wait=sb ls-lb-del sw0 lb1 > +check ovn-nbctl lb-add lb2 10.0.0.5 10.0.0.2,10.0.0.3 > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb2 > + > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Set hairpin_snat_ip to Load Balancer without port.]) > +check ovn-nbctl --wait=sb set load_balancer lb2 options:hairpin_snat_ip="10.0.0.6" > + > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.6), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Create one more logical switch with load balancer.]) > +# Check if load balancer is attached to multiple logical switches: > +# the lflows for the load balancer will only change on the switch > +# where the option is enabled. > +check ovn-nbctl ls-add sw1 > +check ovn-nbctl ls-lb-add sw1 lb2 > + > +ovn-sbctl dump-flows sw1 > sw1flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw1flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > +]) > + > +AT_CLEANUP > +]) > + > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 1dccefb09..dbd129e3b 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -5184,6 +5184,97 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([system enable-stateless-acl-with-lb usage]) Nit: We should add a "AT_SKIP_IF([test $HAVE_NC = no])" here. > + > +ovn_start > + > +OVS_TRAFFIC_VSWITCHD_START() > +ADD_BR([br-int]) > + > +# Set external-ids in br-int needed for ovn-controller > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true > + > +# Start ovn-controller > +start_daemon ovn-controller > + > +# Logical network: > +# One logical switch with IPv4 load balancers that hairpin the traffic. > +check ovn-nbctl ls-add sw > +check ovn-nbctl lsp-add sw lsp1 -- lsp-set-addresses lsp1 00:00:00:00:00:01 > +check ovn-nbctl lsp-add sw lsp2 -- lsp-set-addresses lsp2 00:00:00:00:00:02 > + > +check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp > +check ovn-nbctl ls-lb-add sw lb-ipv4-tcp > + > +check ovn-nbctl lr-add rtr > +check ovn-nbctl lrp-add rtr rtr-sw 00:00:00:00:01:00 42.42.42.254/24 > +check ovn-nbctl lsp-add sw sw-rtr \ > + -- lsp-set-type sw-rtr router \ > + -- lsp-set-addresses sw-rtr 00:00:00:00:01:00 \ > + -- lsp-set-options sw-rtr router-port=rtr-sw > + > +ADD_NAMESPACES(lsp1) > +ADD_VETH(lsp1, lsp1, br-int, "42.42.42.1/24", "00:00:00:00:00:01", \ > + "42.42.42.254") > + > +ADD_NAMESPACES(lsp2) > +ADD_VETH(lsp2, lsp2, br-int, "42.42.42.2/24", "00:00:00:00:00:02", \ > + "42.42.42.254") > + > +# Wait for ovn-controller to catch up. > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +# Start IPv4 TCP server on lsp1. > +NETNS_DAEMONIZE([lsp1], [nc -l -k 42.42.42.1 4041], [lsp1.pid]) > + > +# Send the packet to VIP. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +check ovn-nbctl --wait=hv acl-add sw to-lport 2000 'ip' allow-stateless > +check ovn-nbctl --wait=hv acl-add sw from-lport 2000 'ip' allow-stateless > + > +# To provide work of load balancer with stateless ACL this is necessary > +# to set enable-stateless-acl-lb to true. > +check ovn-nbctl set logical_switch sw other_config:enable-stateless-acl-with-lb=true > + > +check ovn-nbctl --wait=hv sync > + > +# Send the packet to VIP after add stateless acl. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +check ovn-nbctl --wait=hv acl-add sw to-lport 2001 'ip' allow-related > +check ovn-nbctl --wait=hv acl-add sw from-lport 2001 'ip' allow-related > + > +# Send the packet to VIP after add related acls. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +OVN_CLEANUP_CONTROLLER([hv1]) > + > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as ovn-nb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as northd > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > + > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > +/connection dropped.*/d"]) > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([Load Balancer LS hairpin IPv6]) > AT_SKIP_IF([test $HAVE_NC = no]) To avoid a v6, I took care of the minor comments I had and pushed the patch to main. Thanks a lot for your patience in driving this change to completion! Regards, Dumitru
Hi Dumitru! Many thanks for your time spent on this important patch for us 😊 -- Regards, Vladislav Odintsov From: dev <ovs-dev-bounces@openvswitch.org> on behalf of Dumitru Ceara via dev <ovs-dev@openvswitch.org> Date: Wednesday, 21 May 2025 at 14:53 To: Rukomoinikova Aleksandra <ARukomoinikova@k2.cloud>, dev@openvswitch.org <dev@openvswitch.org> Cc: Venugopal Iyer <venugopali@nvidia.com> Subject: Re: [ovs-dev] [PATCH ovn v5] northd: Add option to make work lb with stateless ACL. On 5/15/25 1:59 PM, Alexandra Rukomoinikova wrote: > [0] Removed support for using load balancers in conjunction with stateless ACL. > This commit adds an option enable-stateless-acl-with-lb for logical switch to > make load balancers work. > > [0] - https://github.com/ovn-org/ovn/commit/a0f82efdd9dfd3ef2d9606c1890e353df1097a51 > > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > --- > v4 --> v5: 1) made this functionality optional on the logical switch. > 2) moved snat_ip to ovn_northd_lb structure from lb_vip bc > it's seems to be more correct. > 3) added ls_lb_with_stateless_mode hmapx in ovn_lb_datapaths > because the flows for the load balancer are added to the datapath group, > and I would like to separately add the flows needed for the balancers > to work with stateless acl only for datapath with enabled option. > 4) added doc and NEWS. > --- Hi Alexandra, Thanks for v5! > NEWS | 3 + > northd/lb.c | 18 +++++- > northd/lb.h | 5 ++ > northd/northd.c | 62 +++++++++++++++++++- > northd/northd.h | 4 ++ > northd/ovn-northd.8.xml | 8 +++ > ovn-nb.xml | 8 +++ > tests/ovn-northd.at | 123 ++++++++++++++++++++++++++++++++++++++++ > tests/system-ovn.at | 91 +++++++++++++++++++++++++++++ > 9 files changed, 318 insertions(+), 4 deletions(-) > > diff --git a/NEWS b/NEWS > index f82f25754..52c829e83 100644 > --- a/NEWS > +++ b/NEWS > @@ -20,6 +20,9 @@ Post v25.03.0 > - Introduce exclude-router-ips-from-garp in logical_switch_port column > so that the router port IPs are not advertised in the GARPs. > - Added support for port mirroring in OVN overlay. > + - Add a new logical switch option - enable-stateless-acl-lb with default > + value of false. This option should be set to true for logical switch Nit: s/switch/switches/ > + with stateless ACL to work with load balancer. > > OVN v25.03.0 - 07 Mar 2025 > -------------------------- > diff --git a/northd/lb.c b/northd/lb.c > index ffd21c5bc..b11896cf1 100644 > --- a/northd/lb.c > +++ b/northd/lb.c > @@ -257,6 +257,14 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb, > return NULL; > } > > +static bool > +validate_snap_ip_address(const char *snat_ip) > +{ > + ovs_be32 ip; > + > + return ip_parse(snat_ip, &ip); > +} > + > static void > ovn_northd_lb_init(struct ovn_northd_lb *lb, > const struct nbrec_load_balancer *nbrec_lb) > @@ -297,6 +305,12 @@ ovn_northd_lb_init(struct ovn_northd_lb *lb, > } > lb->affinity_timeout = affinity_timeout; > > + const char *snat_ip = smap_get(&nbrec_lb->options, "hairpin_snat_ip"); > + > + if (snat_ip && validate_snap_ip_address(snat_ip)) { > + lb->hairpin_snat_ip = xstrdup(snat_ip); > + } > + > sset_init(&lb->ips_v4); > sset_init(&lb->ips_v6); > struct smap_node *node; > @@ -413,6 +427,7 @@ ovn_northd_lb_cleanup(struct ovn_northd_lb *lb) > sset_destroy(&lb->ips_v4); > sset_destroy(&lb->ips_v6); > free(lb->selection_fields); > + free(lb->hairpin_snat_ip); > lb->selection_fields = NULL; > lb->health_checks = false; > } > @@ -566,7 +581,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths, > lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths); > lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths); > lb_dps->lflow_ref = lflow_ref_create(); > - > + hmapx_init(&lb_dps->ls_lb_with_stateless_mode); > return lb_dps; > } > > @@ -576,6 +591,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps) > bitmap_free(lb_dps->nb_lr_map); > bitmap_free(lb_dps->nb_ls_map); > lflow_ref_destroy(lb_dps->lflow_ref); > + hmapx_destroy(&lb_dps->ls_lb_with_stateless_mode); > free(lb_dps); > } > > diff --git a/northd/lb.h b/northd/lb.h > index aa6616af4..eb1942bd4 100644 > --- a/northd/lb.h > +++ b/northd/lb.h > @@ -18,6 +18,7 @@ > #define OVN_NORTHD_LB_H 1 > > #include "openvswitch/hmap.h" > +#include "hmapx.h" > #include "uuid.h" > > #include "lib/lb.h" > @@ -72,6 +73,8 @@ struct ovn_northd_lb { > > /* Indicates if the load balancer has health checks configured. */ > bool health_checks; > + > + char *hairpin_snat_ip; > }; > > /* ovn-northd specific backend information. */ > @@ -139,6 +142,8 @@ struct ovn_lb_datapaths { > size_t n_nb_lr; > unsigned long *nb_lr_map; > > + struct hmapx ls_lb_with_stateless_mode; > + > /* Reference of lflows generated for this load balancer. > * > * This data is initialized and destroyed by the en_northd node, but > diff --git a/northd/northd.c b/northd/northd.c > index 7b05147b4..85a1fcc58 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -503,6 +503,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, > od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); > + od->lb_with_stateless_mode = false; > return od; > } > > @@ -942,6 +943,12 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, > if (smap_get_bool(&nbs->other_config, "ic-vxlan_mode", false)) { > vxlan_ic_mode = true; > } > + > + if (smap_get_bool(&nbs->other_config, > + "enable-stateless-acl-with-lb", false)) { Nit: indentation is a bit off. > + VLOG_WARN("set option to true"); This VLOG_WARN should be removed. > + od->lb_with_stateless_mode = true; > + } > } > > const struct nbrec_logical_router *nbr; > @@ -3965,6 +3972,9 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, > lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); > ovs_assert(lb_dps); > ovn_lb_datapaths_add_ls(lb_dps, 1, &od); > + if (od->lb_with_stateless_mode) { > + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); > + } > } > > for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { > @@ -5534,6 +5544,10 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, > ovs_assert(lb_dps); > ovn_lb_datapaths_add_ls(lb_dps, 1, &od); > > + if (od->lb_with_stateless_mode) { > + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); > + } > + > /* Add the lb to the northd tracked data. */ > hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); > } > @@ -6215,7 +6229,7 @@ build_stateless_filter(const struct ovn_datapath *od, > action, > &acl->header_, > lflow_ref); > - } else { > + } else if (!od->lb_with_stateless_mode) { > ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL, > acl->priority + OVN_ACL_PRI_OFFSET, > acl->match, > @@ -7780,8 +7794,14 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, > * > * 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 || " : ""); > + > + if (use_ct_inv_match && !od->lb_with_stateless_mode) { > + ds_put_cstr(&match, "ct.inv || (ct.est && ct.rpl && " > + "ct_mark.blocked == 1)"); > + } else { > + ds_put_cstr(&match, "(ct.est && ct.rpl && ct_mark.blocked == 1)"); > + } > + > ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX - 3, > ds_cstr(&match), REGBIT_ACL_VERDICT_DROP " = 1; next;", > lflow_ref); > @@ -8086,6 +8106,42 @@ build_lb_rules_pre_stateful(struct lflow_table *lflows, > lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths), > S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match), ds_cstr(action), > &lb->nlb->header_, lb_dps->lflow_ref); > + > + struct lflow_ref *lflow_ref = lb_dps->lflow_ref; > + struct hmapx_node *hmapx_node; > + struct ovn_datapath *od; > + HMAPX_FOR_EACH (hmapx_node, &lb_dps->ls_lb_with_stateless_mode) { > + od = hmapx_node->data; > + > + ds_clear(action); > + ds_clear(match); > + > + ds_put_format(match, "%s.dst == %s", ip_match, lb_vip->vip_str); > + > + if (lb_vip->port_str) { > + ds_put_format(match, " && %s.dst == %s", lb->proto, > + lb_vip->port_str); Nit: indentation is a bit off. > + } > + > + ds_put_cstr(action, "ct_lb_mark;"); > + > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 150, > + ds_cstr(match), ds_cstr(action), lflow_ref); > + > + if (lb->hairpin_snat_ip || lb_vip->port_str) { > + ds_clear(action); > + ds_clear(match); > + > + ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match, > + lb->hairpin_snat_ip ? lb->hairpin_snat_ip > + : lb_vip->vip_str); I think this matches our style better: ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match, lb->hairpin_snat_ip ? lb->hairpin_snat_ip : lb_vip->vip_str); > + > + ds_put_cstr(action, "ct_lb_mark;"); > + > + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 105, > + ds_cstr(match), ds_cstr(action), lflow_ref); > + } > + } > } > } > > diff --git a/northd/northd.h b/northd/northd.h > index 5a698458f..f32ff1e2e 100644 > --- a/northd/northd.h > +++ b/northd/northd.h > @@ -379,6 +379,10 @@ struct ovn_datapath { > bool has_vtep_lports; > bool has_arp_proxy_port; > > + /* Set to true if the option 'enable-stateless-acl-with-lb' is enabled > + * on the logical switch. */ > + bool lb_with_stateless_mode; > + > /* IPAM data. */ > struct ipam_info ipam_info; > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index 96e102eeb..5d00cff89 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -652,6 +652,12 @@ > <code>ct_lb_mark;</code> action. > </li> > > + <li> > + A priority-105 added enabled when enable-stateless-acl-with-lb and > + send all packet directed to VIP that don't match the above flows to > + connection tracker. > + </li> > + > <li> > A priority-100 flow sends the packets to connection tracker based > on a hint provided by the previous tables > @@ -2331,6 +2337,8 @@ output; > <p> > This is similar to ingress table <code>Pre-ACLs</code> except for > <code>to-lport</code> traffic. > + Except when the option enable-stateless-acl-with-lbis enabled: Typo: enable-stateless-acl-with-lbis enabled > + REGBIT_ACL_STATELESS ignored. > </p> > > <p> > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 4396023a4..94ee92b9c 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -839,6 +839,14 @@ > is not set and the zone limit is derived from OvS default datapath > limit. > </column> > + > + <column name="other_config" key="enable-stateless-acl-with-lb" > + type='{"type": "boolean"}'> > + This option must be set to true for stateless ACL to work with load > + balancers. When enabled, packets with the ct.inv flag will not be > + dropped, even if use_ct_inv_match is set to true. > + Default: <code>false</code>. > + </column> > </group> > > <group title="IP Multicast Snooping Options"> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 69b75fe9d..0eca60fdb 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -17290,3 +17290,126 @@ AT_CHECK([cat trace | grep output], [0], [dnl > > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([enable-stateless-acl-with-lb usage]) > +ovn_start ovn-northd > + > +AS_BOX([Create logical switches and ports.]) > +check ovn-nbctl ls-add sw0 > +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \ > +"00:00:00:00:00:02 10.0.0.2" > + > +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 \ > +"00:00:00:00:00:03 10.0.0.3" > + > +AS_BOX([Create stateless ACLs.]) > +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1001 "ip" allow-stateless > +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1001 "ip" allow-stateless > + > +AS_BOX([Create stateful ACLs.]) > +# check if allow-stateless acls have higher priority we skip conntrack. > +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1000 "ip" allow-related > +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1000 "ip" allow-related > + > +AS_BOX([Create Load Balancer.]) > +check ovn-nbctl lb-add lb1 10.0.0.4:80 10.0.0.2:80,10.0.0.3:80 > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 > + > +ovn-sbctl dump-flows sw0 > sw0flows > + > +AT_CHECK( > + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > +]) > + > +AT_CHECK( > + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl > + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) This part needs a small rebase since: 8828b6120ea1 ("northd: Remove redundant ct state matches.") https://github.com/ovn-org/ovn/commit/8828b61 > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.inv || (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > +]) > + > +AS_BOX([Enable enable-stateless-acl-with-lb option.]) > +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:enable-stateless-acl-with-lb=true > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK( > + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) > + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) > +]) > + > +# We do not match conntrack invalide packets in case of load balancers with stateless ACLs. Typo: invalide > +AT_CHECK( > + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl > + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) > + table=??(ls_out_acl_eval ), priority=65532, match=((ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) > + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) > +]) > + > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.4), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(reg4 = 10.0.0.4; reg2[[0..15]] = 80; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Create Load Balancer without port.]) > +check ovn-nbctl --wait=sb ls-lb-del sw0 lb1 > +check ovn-nbctl lb-add lb2 10.0.0.5 10.0.0.2,10.0.0.3 > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb2 > + > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Set hairpin_snat_ip to Load Balancer without port.]) > +check ovn-nbctl --wait=sb set load_balancer lb2 options:hairpin_snat_ip="10.0.0.6" > + > +ovn-sbctl dump-flows sw0 > sw0flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.6), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) > +]) > + > +AS_BOX([Create one more logical switch with load balancer.]) > +# Check if load balancer is attached to multiple logical switches: > +# the lflows for the load balancer will only change on the switch > +# where the option is enabled. > +check ovn-nbctl ls-add sw1 > +check ovn-nbctl ls-lb-add sw1 lb2 > + > +ovn-sbctl dump-flows sw1 > sw1flows > +AT_CHECK([grep -E 'ls_in_pre_stateful' sw1flows | ovn_strip_lflows], [0], [dnl > + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) > + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) > + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) > + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) > +]) > + > +AT_CLEANUP > +]) > + > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > index 1dccefb09..dbd129e3b 100644 > --- a/tests/system-ovn.at > +++ b/tests/system-ovn.at > @@ -5184,6 +5184,97 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([system enable-stateless-acl-with-lb usage]) Nit: We should add a "AT_SKIP_IF([test $HAVE_NC = no])" here. > + > +ovn_start > + > +OVS_TRAFFIC_VSWITCHD_START() > +ADD_BR([br-int]) > + > +# Set external-ids in br-int needed for ovn-controller > +ovs-vsctl \ > + -- set Open_vSwitch . external-ids:system-id=hv1 \ > + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ > + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ > + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ > + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true > + > +# Start ovn-controller > +start_daemon ovn-controller > + > +# Logical network: > +# One logical switch with IPv4 load balancers that hairpin the traffic. > +check ovn-nbctl ls-add sw > +check ovn-nbctl lsp-add sw lsp1 -- lsp-set-addresses lsp1 00:00:00:00:00:01 > +check ovn-nbctl lsp-add sw lsp2 -- lsp-set-addresses lsp2 00:00:00:00:00:02 > + > +check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp > +check ovn-nbctl ls-lb-add sw lb-ipv4-tcp > + > +check ovn-nbctl lr-add rtr > +check ovn-nbctl lrp-add rtr rtr-sw 00:00:00:00:01:00 42.42.42.254/24 > +check ovn-nbctl lsp-add sw sw-rtr \ > + -- lsp-set-type sw-rtr router \ > + -- lsp-set-addresses sw-rtr 00:00:00:00:01:00 \ > + -- lsp-set-options sw-rtr router-port=rtr-sw > + > +ADD_NAMESPACES(lsp1) > +ADD_VETH(lsp1, lsp1, br-int, "42.42.42.1/24", "00:00:00:00:00:01", \ > + "42.42.42.254") > + > +ADD_NAMESPACES(lsp2) > +ADD_VETH(lsp2, lsp2, br-int, "42.42.42.2/24", "00:00:00:00:00:02", \ > + "42.42.42.254") > + > +# Wait for ovn-controller to catch up. > +wait_for_ports_up > +check ovn-nbctl --wait=hv sync > + > +# Start IPv4 TCP server on lsp1. > +NETNS_DAEMONIZE([lsp1], [nc -l -k 42.42.42.1 4041], [lsp1.pid]) > + > +# Send the packet to VIP. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +check ovn-nbctl --wait=hv acl-add sw to-lport 2000 'ip' allow-stateless > +check ovn-nbctl --wait=hv acl-add sw from-lport 2000 'ip' allow-stateless > + > +# To provide work of load balancer with stateless ACL this is necessary > +# to set enable-stateless-acl-lb to true. > +check ovn-nbctl set logical_switch sw other_config:enable-stateless-acl-with-lb=true > + > +check ovn-nbctl --wait=hv sync > + > +# Send the packet to VIP after add stateless acl. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +check ovn-nbctl --wait=hv acl-add sw to-lport 2001 'ip' allow-related > +check ovn-nbctl --wait=hv acl-add sw from-lport 2001 'ip' allow-related > + > +# Send the packet to VIP after add related acls. > +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) > + > +OVN_CLEANUP_CONTROLLER([hv1]) > + > +as ovn-sb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as ovn-nb > +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) > + > +as northd > +OVS_APP_EXIT_AND_WAIT([ovn-northd]) > + > +as > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d > +/connection dropped.*/d"]) > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD([ > AT_SETUP([Load Balancer LS hairpin IPv6]) > AT_SKIP_IF([test $HAVE_NC = no]) To avoid a v6, I took care of the minor comments I had and pushed the patch to main. Thanks a lot for your patience in driving this change to completion! Regards, Dumitru
diff --git a/NEWS b/NEWS index f82f25754..52c829e83 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,9 @@ Post v25.03.0 - Introduce exclude-router-ips-from-garp in logical_switch_port column so that the router port IPs are not advertised in the GARPs. - Added support for port mirroring in OVN overlay. + - Add a new logical switch option - enable-stateless-acl-lb with default + value of false. This option should be set to true for logical switch + with stateless ACL to work with load balancer. OVN v25.03.0 - 07 Mar 2025 -------------------------- diff --git a/northd/lb.c b/northd/lb.c index ffd21c5bc..b11896cf1 100644 --- a/northd/lb.c +++ b/northd/lb.c @@ -257,6 +257,14 @@ ovn_lb_get_health_check(const struct nbrec_load_balancer *nbrec_lb, return NULL; } +static bool +validate_snap_ip_address(const char *snat_ip) +{ + ovs_be32 ip; + + return ip_parse(snat_ip, &ip); +} + static void ovn_northd_lb_init(struct ovn_northd_lb *lb, const struct nbrec_load_balancer *nbrec_lb) @@ -297,6 +305,12 @@ ovn_northd_lb_init(struct ovn_northd_lb *lb, } lb->affinity_timeout = affinity_timeout; + const char *snat_ip = smap_get(&nbrec_lb->options, "hairpin_snat_ip"); + + if (snat_ip && validate_snap_ip_address(snat_ip)) { + lb->hairpin_snat_ip = xstrdup(snat_ip); + } + sset_init(&lb->ips_v4); sset_init(&lb->ips_v6); struct smap_node *node; @@ -413,6 +427,7 @@ ovn_northd_lb_cleanup(struct ovn_northd_lb *lb) sset_destroy(&lb->ips_v4); sset_destroy(&lb->ips_v6); free(lb->selection_fields); + free(lb->hairpin_snat_ip); lb->selection_fields = NULL; lb->health_checks = false; } @@ -566,7 +581,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths, lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths); lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths); lb_dps->lflow_ref = lflow_ref_create(); - + hmapx_init(&lb_dps->ls_lb_with_stateless_mode); return lb_dps; } @@ -576,6 +591,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps) bitmap_free(lb_dps->nb_lr_map); bitmap_free(lb_dps->nb_ls_map); lflow_ref_destroy(lb_dps->lflow_ref); + hmapx_destroy(&lb_dps->ls_lb_with_stateless_mode); free(lb_dps); } diff --git a/northd/lb.h b/northd/lb.h index aa6616af4..eb1942bd4 100644 --- a/northd/lb.h +++ b/northd/lb.h @@ -18,6 +18,7 @@ #define OVN_NORTHD_LB_H 1 #include "openvswitch/hmap.h" +#include "hmapx.h" #include "uuid.h" #include "lib/lb.h" @@ -72,6 +73,8 @@ struct ovn_northd_lb { /* Indicates if the load balancer has health checks configured. */ bool health_checks; + + char *hairpin_snat_ip; }; /* ovn-northd specific backend information. */ @@ -139,6 +142,8 @@ struct ovn_lb_datapaths { size_t n_nb_lr; unsigned long *nb_lr_map; + struct hmapx ls_lb_with_stateless_mode; + /* Reference of lflows generated for this load balancer. * * This data is initialized and destroyed by the en_northd node, but diff --git a/northd/northd.c b/northd/northd.c index 7b05147b4..85a1fcc58 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -503,6 +503,7 @@ ovn_datapath_create(struct hmap *datapaths, const struct uuid *key, od->router_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); od->l3dgw_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); od->localnet_ports = VECTOR_EMPTY_INITIALIZER(struct ovn_port *); + od->lb_with_stateless_mode = false; return od; } @@ -942,6 +943,12 @@ join_datapaths(const struct nbrec_logical_switch_table *nbrec_ls_table, if (smap_get_bool(&nbs->other_config, "ic-vxlan_mode", false)) { vxlan_ic_mode = true; } + + if (smap_get_bool(&nbs->other_config, + "enable-stateless-acl-with-lb", false)) { + VLOG_WARN("set option to true"); + od->lb_with_stateless_mode = true; + } } const struct nbrec_logical_router *nbr; @@ -3965,6 +3972,9 @@ build_lb_datapaths(const struct hmap *lbs, const struct hmap *lb_groups, lb_dps = ovn_lb_datapaths_find(lb_datapaths_map, lb_uuid); ovs_assert(lb_dps); ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + if (od->lb_with_stateless_mode) { + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); + } } for (size_t i = 0; i < od->nbs->n_load_balancer_group; i++) { @@ -5534,6 +5544,10 @@ northd_handle_lb_data_changes(struct tracked_lb_data *trk_lb_data, ovs_assert(lb_dps); ovn_lb_datapaths_add_ls(lb_dps, 1, &od); + if (od->lb_with_stateless_mode) { + hmapx_add(&lb_dps->ls_lb_with_stateless_mode, od); + } + /* Add the lb to the northd tracked data. */ hmapx_add(&nd_changes->trk_lbs.crupdated, lb_dps); } @@ -6215,7 +6229,7 @@ build_stateless_filter(const struct ovn_datapath *od, action, &acl->header_, lflow_ref); - } else { + } else if (!od->lb_with_stateless_mode) { ovn_lflow_add_with_hint(lflows, od, S_SWITCH_OUT_PRE_ACL, acl->priority + OVN_ACL_PRI_OFFSET, acl->match, @@ -7780,8 +7794,14 @@ build_acls(const struct ls_stateful_record *ls_stateful_rec, * * 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 || " : ""); + + if (use_ct_inv_match && !od->lb_with_stateless_mode) { + ds_put_cstr(&match, "ct.inv || (ct.est && ct.rpl && " + "ct_mark.blocked == 1)"); + } else { + ds_put_cstr(&match, "(ct.est && ct.rpl && ct_mark.blocked == 1)"); + } + ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_EVAL, UINT16_MAX - 3, ds_cstr(&match), REGBIT_ACL_VERDICT_DROP " = 1; next;", lflow_ref); @@ -8086,6 +8106,42 @@ build_lb_rules_pre_stateful(struct lflow_table *lflows, lflows, lb_dps->nb_ls_map, ods_size(ls_datapaths), S_SWITCH_IN_PRE_STATEFUL, 120, ds_cstr(match), ds_cstr(action), &lb->nlb->header_, lb_dps->lflow_ref); + + struct lflow_ref *lflow_ref = lb_dps->lflow_ref; + struct hmapx_node *hmapx_node; + struct ovn_datapath *od; + HMAPX_FOR_EACH (hmapx_node, &lb_dps->ls_lb_with_stateless_mode) { + od = hmapx_node->data; + + ds_clear(action); + ds_clear(match); + + ds_put_format(match, "%s.dst == %s", ip_match, lb_vip->vip_str); + + if (lb_vip->port_str) { + ds_put_format(match, " && %s.dst == %s", lb->proto, + lb_vip->port_str); + } + + ds_put_cstr(action, "ct_lb_mark;"); + + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 150, + ds_cstr(match), ds_cstr(action), lflow_ref); + + if (lb->hairpin_snat_ip || lb_vip->port_str) { + ds_clear(action); + ds_clear(match); + + ds_put_format(match, "%s && %s.dst == %s", lb->proto, ip_match, + lb->hairpin_snat_ip ? lb->hairpin_snat_ip + : lb_vip->vip_str); + + ds_put_cstr(action, "ct_lb_mark;"); + + ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_STATEFUL, 105, + ds_cstr(match), ds_cstr(action), lflow_ref); + } + } } } diff --git a/northd/northd.h b/northd/northd.h index 5a698458f..f32ff1e2e 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -379,6 +379,10 @@ struct ovn_datapath { bool has_vtep_lports; bool has_arp_proxy_port; + /* Set to true if the option 'enable-stateless-acl-with-lb' is enabled + * on the logical switch. */ + bool lb_with_stateless_mode; + /* IPAM data. */ struct ipam_info ipam_info; diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index 96e102eeb..5d00cff89 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -652,6 +652,12 @@ <code>ct_lb_mark;</code> action. </li> + <li> + A priority-105 added enabled when enable-stateless-acl-with-lb and + send all packet directed to VIP that don't match the above flows to + connection tracker. + </li> + <li> A priority-100 flow sends the packets to connection tracker based on a hint provided by the previous tables @@ -2331,6 +2337,8 @@ output; <p> This is similar to ingress table <code>Pre-ACLs</code> except for <code>to-lport</code> traffic. + Except when the option enable-stateless-acl-with-lbis enabled: + REGBIT_ACL_STATELESS ignored. </p> <p> diff --git a/ovn-nb.xml b/ovn-nb.xml index 4396023a4..94ee92b9c 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -839,6 +839,14 @@ is not set and the zone limit is derived from OvS default datapath limit. </column> + + <column name="other_config" key="enable-stateless-acl-with-lb" + type='{"type": "boolean"}'> + This option must be set to true for stateless ACL to work with load + balancers. When enabled, packets with the ct.inv flag will not be + dropped, even if use_ct_inv_match is set to true. + Default: <code>false</code>. + </column> </group> <group title="IP Multicast Snooping Options"> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 69b75fe9d..0eca60fdb 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -17290,3 +17290,126 @@ AT_CHECK([cat trace | grep output], [0], [dnl AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([enable-stateless-acl-with-lb usage]) +ovn_start ovn-northd + +AS_BOX([Create logical switches and ports.]) +check ovn-nbctl ls-add sw0 +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p1 -- lsp-set-addresses sw0-p1 \ +"00:00:00:00:00:02 10.0.0.2" + +check ovn-nbctl --wait=sb lsp-add sw0 sw0-p2 -- lsp-set-addresses sw0-p2 \ +"00:00:00:00:00:03 10.0.0.3" + +AS_BOX([Create stateless ACLs.]) +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1001 "ip" allow-stateless +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1001 "ip" allow-stateless + +AS_BOX([Create stateful ACLs.]) +# check if allow-stateless acls have higher priority we skip conntrack. +check ovn-nbctl --wait=sb acl-add sw0 from-lport 1000 "ip" allow-related +check ovn-nbctl --wait=sb acl-add sw0 to-lport 1000 "ip" allow-related + +AS_BOX([Create Load Balancer.]) +check ovn-nbctl lb-add lb1 10.0.0.4:80 10.0.0.2:80,10.0.0.3:80 +check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 + +ovn-sbctl dump-flows sw0 > sw0flows + +AT_CHECK( + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) + table=??(ls_out_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) +]) + +AT_CHECK( + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(ct.inv || (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) +]) + +AS_BOX([Enable enable-stateless-acl-with-lb option.]) +check ovn-nbctl --wait=sb set logical_switch sw0 other_config:enable-stateless-acl-with-lb=true +ovn-sbctl dump-flows sw0 > sw0flows +AT_CHECK( + [grep -E 'ls_(in|out)_pre_acl' sw0flows | grep reg0 | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) + table=??(ls_in_pre_acl ), priority=2001 , match=(ip), action=(reg0[[16]] = 1; next;) + table=??(ls_out_pre_acl ), priority=100 , match=(ip), action=(reg0[[0]] = 1; next;) +]) + +# We do not match conntrack invalide packets in case of load balancers with stateless ACLs. +AT_CHECK( + [grep -E 'ls_out_acl_eval' sw0flows | grep 65532 | ovn_strip_lflows], [0], [dnl + table=??(ls_out_acl_eval ), priority=65532, match=(!ct.est && ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg8[[16]] = 1; ct_commit_nat;) + table=??(ls_out_acl_eval ), priority=65532, match=((ct.est && ct.rpl && ct_mark.blocked == 1)), action=(reg8[[17]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0), action=(reg8[[16]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(ct.est && ct_mark.allow_established == 1), action=(reg8[[16]] = 1; next;) + table=??(ls_out_acl_eval ), priority=65532, match=(nd || nd_ra || nd_rs || mldv1 || mldv2), action=(reg8[[16]] = 1; next;) +]) + +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.4), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(reg4 = 10.0.0.4; reg2[[0..15]] = 80; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.4 && tcp.dst == 80), action=(ct_lb_mark;) +]) + +AS_BOX([Create Load Balancer without port.]) +check ovn-nbctl --wait=sb ls-lb-del sw0 lb1 +check ovn-nbctl lb-add lb2 10.0.0.5 10.0.0.2,10.0.0.3 +check ovn-nbctl --wait=sb ls-lb-add sw0 lb2 + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) +]) + +AS_BOX([Set hairpin_snat_ip to Load Balancer without port.]) +check ovn-nbctl --wait=sb set load_balancer lb2 options:hairpin_snat_ip="10.0.0.6" + +ovn-sbctl dump-flows sw0 > sw0flows +AT_CHECK([grep -E 'ls_in_pre_stateful' sw0flows | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) + table=??(ls_in_pre_stateful ), priority=105 , match=(tcp && ip4.dst == 10.0.0.6), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=150 , match=(ip4.dst == 10.0.0.5), action=(ct_lb_mark;) +]) + +AS_BOX([Create one more logical switch with load balancer.]) +# Check if load balancer is attached to multiple logical switches: +# the lflows for the load balancer will only change on the switch +# where the option is enabled. +check ovn-nbctl ls-add sw1 +check ovn-nbctl ls-lb-add sw1 lb2 + +ovn-sbctl dump-flows sw1 > sw1flows +AT_CHECK([grep -E 'ls_in_pre_stateful' sw1flows | ovn_strip_lflows], [0], [dnl + table=??(ls_in_pre_stateful ), priority=0 , match=(1), action=(next;) + table=??(ls_in_pre_stateful ), priority=100 , match=(reg0[[0]] == 1), action=(ct_next;) + table=??(ls_in_pre_stateful ), priority=110 , match=(reg0[[2]] == 1), action=(ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=115 , match=(reg0[[2]] == 1 && ip.is_frag), action=(reg0[[19]] = 1; ct_lb_mark;) + table=??(ls_in_pre_stateful ), priority=120 , match=(reg0[[2]] == 1 && ip4.dst == 10.0.0.5), action=(reg4 = 10.0.0.5; ct_lb_mark;) +]) + +AT_CLEANUP +]) + diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 1dccefb09..dbd129e3b 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -5184,6 +5184,97 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([system enable-stateless-acl-with-lb usage]) + +ovn_start + +OVS_TRAFFIC_VSWITCHD_START() +ADD_BR([br-int]) + +# Set external-ids in br-int needed for ovn-controller +ovs-vsctl \ + -- set Open_vSwitch . external-ids:system-id=hv1 \ + -- set Open_vSwitch . external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \ + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \ + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \ + -- set bridge br-int fail-mode=secure other-config:disable-in-band=true + +# Start ovn-controller +start_daemon ovn-controller + +# Logical network: +# One logical switch with IPv4 load balancers that hairpin the traffic. +check ovn-nbctl ls-add sw +check ovn-nbctl lsp-add sw lsp1 -- lsp-set-addresses lsp1 00:00:00:00:00:01 +check ovn-nbctl lsp-add sw lsp2 -- lsp-set-addresses lsp2 00:00:00:00:00:02 + +check ovn-nbctl lb-add lb-ipv4-tcp 88.88.88.88:8080 42.42.42.1:4041 tcp +check ovn-nbctl ls-lb-add sw lb-ipv4-tcp + +check ovn-nbctl lr-add rtr +check ovn-nbctl lrp-add rtr rtr-sw 00:00:00:00:01:00 42.42.42.254/24 +check ovn-nbctl lsp-add sw sw-rtr \ + -- lsp-set-type sw-rtr router \ + -- lsp-set-addresses sw-rtr 00:00:00:00:01:00 \ + -- lsp-set-options sw-rtr router-port=rtr-sw + +ADD_NAMESPACES(lsp1) +ADD_VETH(lsp1, lsp1, br-int, "42.42.42.1/24", "00:00:00:00:00:01", \ + "42.42.42.254") + +ADD_NAMESPACES(lsp2) +ADD_VETH(lsp2, lsp2, br-int, "42.42.42.2/24", "00:00:00:00:00:02", \ + "42.42.42.254") + +# Wait for ovn-controller to catch up. +wait_for_ports_up +check ovn-nbctl --wait=hv sync + +# Start IPv4 TCP server on lsp1. +NETNS_DAEMONIZE([lsp1], [nc -l -k 42.42.42.1 4041], [lsp1.pid]) + +# Send the packet to VIP. +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) + +check ovn-nbctl --wait=hv acl-add sw to-lport 2000 'ip' allow-stateless +check ovn-nbctl --wait=hv acl-add sw from-lport 2000 'ip' allow-stateless + +# To provide work of load balancer with stateless ACL this is necessary +# to set enable-stateless-acl-lb to true. +check ovn-nbctl set logical_switch sw other_config:enable-stateless-acl-with-lb=true + +check ovn-nbctl --wait=hv sync + +# Send the packet to VIP after add stateless acl. +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) + +check ovn-nbctl --wait=hv acl-add sw to-lport 2001 'ip' allow-related +check ovn-nbctl --wait=hv acl-add sw from-lport 2001 'ip' allow-related + +# Send the packet to VIP after add related acls. +NS_CHECK_EXEC([lsp1], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) +NS_CHECK_EXEC([lsp2], [nc -z 88.88.88.88 8080], [0], [ignore], [ignore]) + +OVN_CLEANUP_CONTROLLER([hv1]) + +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +as ovn-nb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) + +as northd +OVS_APP_EXIT_AND_WAIT([ovn-northd]) + +as +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d +/connection dropped.*/d"]) +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([Load Balancer LS hairpin IPv6]) AT_SKIP_IF([test $HAVE_NC = no])
[0] Removed support for using load balancers in conjunction with stateless ACL. This commit adds an option enable-stateless-acl-with-lb for logical switch to make load balancers work. [0] - https://github.com/ovn-org/ovn/commit/a0f82efdd9dfd3ef2d9606c1890e353df1097a51 Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> --- v4 --> v5: 1) made this functionality optional on the logical switch. 2) moved snat_ip to ovn_northd_lb structure from lb_vip bc it's seems to be more correct. 3) added ls_lb_with_stateless_mode hmapx in ovn_lb_datapaths because the flows for the load balancer are added to the datapath group, and I would like to separately add the flows needed for the balancers to work with stateless acl only for datapath with enabled option. 4) added doc and NEWS. --- NEWS | 3 + northd/lb.c | 18 +++++- northd/lb.h | 5 ++ northd/northd.c | 62 +++++++++++++++++++- northd/northd.h | 4 ++ northd/ovn-northd.8.xml | 8 +++ ovn-nb.xml | 8 +++ tests/ovn-northd.at | 123 ++++++++++++++++++++++++++++++++++++++++ tests/system-ovn.at | 91 +++++++++++++++++++++++++++++ 9 files changed, 318 insertions(+), 4 deletions(-)