| Message ID | eae604c0fcaf7c7e6caf3189173eb2d9a16e680f.1617983315.git.lorenzo.bianconi@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,v3] northd: introduce per-lb lb_skip_snat option | expand |
Thanks Lorenzo, I pushed this to master. On 4/9/21 11:50 AM, Lorenzo Bianconi wrote: > Introduce skip_snat in load balancer option column in order to not > force_snat traffic that is hitting a given load balancer applied on a > logical router where lb_force_snat has been configured > > https://bugzilla.redhat.com/show_bug.cgi?id=1927540 > > Tested-by: Andrew Stoycos <astoycos@redhat.com> > Acked-by: Mark Michelson <mmichels@redhat.com> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com> > --- > Changes since v2: > - fix typo in ovn-nb.xml > > Changes since v1: > - rename lb_skip_snat in skip_snat > - add skip_snat documentation in ovn-nb.xml > - fix typos > - introduce enum for return value of snat_for_lb/ > --- > include/ovn/logical-fields.h | 5 ++++ > lib/logical-fields.c | 4 +++ > northd/lrouter.dl | 14 ++++++++-- > northd/ovn-northd.8.xml | 24 ++++++++++++++++- > northd/ovn-northd.c | 51 +++++++++++++++++++++++++----------- > northd/ovn_northd.dl | 32 ++++++++++++++++------ > ovn-nb.xml | 6 +++++ > ovs | 2 +- > tests/ovn-northd.at | 25 +++++++++++++++++- > 9 files changed, 135 insertions(+), 28 deletions(-) > > diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h > index 017176f98..d44b30b30 100644 > --- a/include/ovn/logical-fields.h > +++ b/include/ovn/logical-fields.h > @@ -66,6 +66,7 @@ enum mff_log_flags_bits { > MLF_LOOKUP_MAC_BIT = 6, > MLF_LOOKUP_LB_HAIRPIN_BIT = 7, > MLF_LOOKUP_FDB_BIT = 8, > + MLF_SKIP_SNAT_FOR_LB_BIT = 9, > }; > > /* MFF_LOG_FLAGS_REG flag assignments */ > @@ -102,6 +103,10 @@ enum mff_log_flags { > > /* Indicate that the lookup in the fdb table was successful. */ > MLF_LOOKUP_FDB = (1 << MLF_LOOKUP_FDB_BIT), > + > + /* Indicate that a packet must not SNAT in the gateway router when > + * load-balancing has taken place. */ > + MLF_SKIP_SNAT_FOR_LB = (1 << MLF_SKIP_SNAT_FOR_LB_BIT), > }; > > /* OVN logical fields > diff --git a/lib/logical-fields.c b/lib/logical-fields.c > index 9d08b44c2..72853013e 100644 > --- a/lib/logical-fields.c > +++ b/lib/logical-fields.c > @@ -121,6 +121,10 @@ ovn_init_symtab(struct shash *symtab) > MLF_FORCE_SNAT_FOR_LB_BIT); > expr_symtab_add_subfield(symtab, "flags.force_snat_for_lb", NULL, > flags_str); > + snprintf(flags_str, sizeof flags_str, "flags[%d]", > + MLF_SKIP_SNAT_FOR_LB_BIT); > + expr_symtab_add_subfield(symtab, "flags.skip_snat_for_lb", NULL, > + flags_str); > > /* Connection tracking state. */ > expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false, > diff --git a/northd/lrouter.dl b/northd/lrouter.dl > index 36cedd2dc..312ceb9fc 100644 > --- a/northd/lrouter.dl > +++ b/northd/lrouter.dl > @@ -355,8 +355,18 @@ function lb_force_snat_router_ip(lr_options: Map<string, string>): bool { > lr_options.contains_key("chassis") > } > > -function force_snat_for_lb(lr: nb::Logical_Router): bool { > - not get_force_snat_ip(lr, "lb").is_empty() or lb_force_snat_router_ip(lr.options) > +typedef LBForceSNAT = NoForceSNAT > + | ForceSNAT > + | SkipSNAT > + > +function snat_for_lb(lr: nb::Logical_Router, lb: Ref<nb::Load_Balancer>): LBForceSNAT { > + if (lb.options.get_bool_def("skip_snat", false)) { > + return SkipSNAT > + }; > + if (not get_force_snat_ip(lr, "lb").is_empty() or lb_force_snat_router_ip(lr.options)) { > + return ForceSNAT > + }; > + return NoForceSNAT > } > > /* For each router, collect the set of IPv4 and IPv6 addresses used for SNAT, > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > index a62f5c057..8197aa513 100644 > --- a/northd/ovn-northd.8.xml > +++ b/northd/ovn-northd.8.xml > @@ -2754,7 +2754,11 @@ icmp6 { > (and optional port numbers) to load balance to. If the router is > configured to force SNAT any load-balanced packets, the above action > will be replaced by <code>flags.force_snat_for_lb = 1; > - ct_lb(<var>args</var>);</code>. If health check is enabled, then > + ct_lb(<var>args</var>);</code>. > + If the load balancing rule is configured with <code>skip_snat</code> > + set to true, the above action will be replaced by > + <code>flags.skip_snat_for_lb = 1; ct_lb(<var>args</var>);</code>. > + If health check is enabled, then > <var>args</var> will only contain those endpoints whose service > monitor status entry in <code>OVN_Southbound</code> db is > either <code>online</code> or empty. > @@ -2771,6 +2775,9 @@ icmp6 { > with an action of <code>ct_dnat;</code>. If the router is > configured to force SNAT any load-balanced packets, the above action > will be replaced by <code>flags.force_snat_for_lb = 1; ct_dnat;</code>. > + If the load balancing rule is configured with <code>skip_snat</code> > + set to true, the above action will be replaced by > + <code>flags.skip_snat_for_lb = 1; ct_dnat;</code>. > </li> > > <li> > @@ -2785,6 +2792,9 @@ icmp6 { > to force SNAT any load-balanced packets, the above action will be > replaced by <code>flags.force_snat_for_lb = 1; > ct_lb(<var>args</var>);</code>. > + If the load balancing rule is configured with <code>skip_snat</code> > + set to true, the above action will be replaced by > + <code>flags.skip_snat_for_lb = 1; ct_lb(<var>args</var>);</code>. > </li> > > <li> > @@ -2797,6 +2807,9 @@ icmp6 { > If the router is configured to force SNAT any load-balanced > packets, the above action will be replaced by > <code>flags.force_snat_for_lb = 1; ct_dnat;</code>. > + If the load balancing rule is configured with <code>skip_snat</code> > + set to true, the above action will be replaced by > + <code>flags.skip_snat_for_lb = 1; ct_dnat;</code>. > </li> > > <li> > @@ -3829,6 +3842,15 @@ nd_ns { > </p> > </li> > > + <li> > + <p> > + If a load balancer configured to skip snat has been applied to > + the Gateway router pipeline, a priority-120 flow matches > + <code>flags.skip_snat_for_lb == 1 && ip</code> with an > + action <code>next;</code>. > + </p> > + </li> > + > <li> > <p> > If the Gateway router in the OVN Northbound database has been > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 9839b8c4f..8de106c84 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -8573,10 +8573,16 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, > return true; > } > > +enum lb_snat_type { > + NO_FORCE_SNAT, > + FORCE_SNAT, > + SKIP_SNAT, > +}; > + > static void > add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, > struct ds *match, struct ds *actions, int priority, > - bool force_snat_for_lb, struct ovn_lb_vip *lb_vip, > + enum lb_snat_type snat_type, struct ovn_lb_vip *lb_vip, > const char *proto, struct nbrec_load_balancer *lb, > struct shash *meter_groups, struct sset *nat_entries) > { > @@ -8585,9 +8591,10 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, > > /* A match and actions for new connections. */ > char *new_match = xasprintf("ct.new && %s", ds_cstr(match)); > - if (force_snat_for_lb) { > - char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s", > - ds_cstr(actions)); > + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { > + char *new_actions = xasprintf("flags.%s_snat_for_lb = 1; %s", > + snat_type == SKIP_SNAT ? "skip" : "force", > + ds_cstr(actions)); > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, > new_match, new_actions, &lb->header_); > free(new_actions); > @@ -8598,11 +8605,12 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, > > /* A match and actions for established connections. */ > char *est_match = xasprintf("ct.est && %s", ds_cstr(match)); > - if (force_snat_for_lb) { > + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { > + char *est_actions = xasprintf("flags.%s_snat_for_lb = 1; ct_dnat;", > + snat_type == SKIP_SNAT ? "skip" : "force"); > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, > - est_match, > - "flags.force_snat_for_lb = 1; ct_dnat;", > - &lb->header_); > + est_match, est_actions, &lb->header_); > + free(est_actions); > } else { > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, > est_match, "ct_dnat;", &lb->header_); > @@ -8675,11 +8683,13 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, > ds_put_format(&undnat_match, ") && outport == %s && " > "is_chassis_resident(%s)", od->l3dgw_port->json_key, > od->l3redirect_port->json_key); > - if (force_snat_for_lb) { > + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { > + char *action = xasprintf("flags.%s_snat_for_lb = 1; ct_dnat;", > + snat_type == SKIP_SNAT ? "skip" : "force"); > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120, > - ds_cstr(&undnat_match), > - "flags.force_snat_for_lb = 1; ct_dnat;", > + ds_cstr(&undnat_match), action, > &lb->header_); > + free(action); > } else { > ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120, > ds_cstr(&undnat_match), "ct_dnat;", > @@ -8706,6 +8716,12 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od, > ovn_northd_lb_find(lbs, &nb_lb->header_.uuid); > ovs_assert(lb); > > + bool lb_skip_snat = smap_get_bool(&nb_lb->options, "skip_snat", false); > + if (lb_skip_snat) { > + ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120, > + "flags.skip_snat_for_lb == 1 && ip", "next;"); > + } > + > for (size_t j = 0; j < lb->n_vips; j++) { > struct ovn_lb_vip *lb_vip = &lb->vips[j]; > struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[j]; > @@ -8767,11 +8783,16 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od, > ds_put_format(match, " && is_chassis_resident(%s)", > od->l3redirect_port->json_key); > } > - bool force_snat_for_lb = > - lb_force_snat_ip || od->lb_force_snat_router_ip; > + > + enum lb_snat_type snat_type = NO_FORCE_SNAT; > + if (lb_skip_snat) { > + snat_type = SKIP_SNAT; > + } else if (lb_force_snat_ip || od->lb_force_snat_router_ip) { > + snat_type = FORCE_SNAT; > + } > add_router_lb_flow(lflows, od, match, actions, prio, > - force_snat_for_lb, lb_vip, proto, > - nb_lb, meter_groups, nat_entries); > + snat_type, lb_vip, proto, nb_lb, > + meter_groups, nat_entries); > } > } > sset_destroy(&all_ips); > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl > index 0063021e1..4c02e2d5a 100644 > --- a/northd/ovn_northd.dl > +++ b/northd/ovn_northd.dl > @@ -5367,6 +5367,16 @@ for (&Router(.lr = lr)) { > .external_ids = map_empty()) > } > > +Flow(.logical_datapath = lr, > + .stage = s_ROUTER_OUT_SNAT(), > + .priority = 120, > + .__match = "flags.skip_snat_for_lb == 1 && ip", > + .actions = "next;", > + .external_ids = stage_hint(lb._uuid)) :- > + LogicalRouterLB(lr, lb), > + lb.options.get_bool_def("skip_snat", false) > + . > + > function lrouter_nat_is_stateless(nat: NAT): bool = { > Some{"true"} == nat.nat.options.get("stateless") > } > @@ -6010,14 +6020,15 @@ for (RouterLBVIP( > (Some{gwport}, true) -> " && is_chassis_resident(${redirect_port_name})", > _ -> "" > } in > - var force_snat_for_lb = force_snat_for_lb(lr) in > + var snat_for_lb = snat_for_lb(lr, lb) in > { > /* A match and actions for established connections. */ > var est_match = "ct.est && " ++ __match in > var actions = > - match (force_snat_for_lb) { > - true -> "flags.force_snat_for_lb = 1; ct_dnat;", > - false -> "ct_dnat;" > + match (snat_for_lb) { > + SkipSNAT -> "flags.skip_snat_for_lb = 1; ct_dnat;", > + ForceSNAT -> "flags.force_snat_for_lb = 1; ct_dnat;", > + _ -> "ct_dnat;" > } in > Flow(.logical_datapath = lr._uuid, > .stage = s_ROUTER_IN_DNAT(), > @@ -6076,9 +6087,10 @@ for (RouterLBVIP( > ") && outport == ${json_string_escape(gwport.name)} && " > "is_chassis_resident(${redirect_port_name})" in > var action = > - match (force_snat_for_lb) { > - true -> "flags.force_snat_for_lb = 1; ct_dnat;", > - false -> "ct_dnat;" > + match (snat_for_lb) { > + SkipSNAT -> "flags.skip_snat_for_lb = 1; ct_dnat;", > + ForceSNAT -> "flags.force_snat_for_lb = 1; ct_dnat;", > + _ -> "ct_dnat;" > } in > Flow(.logical_datapath = lr._uuid, > .stage = s_ROUTER_OUT_UNDNAT(), > @@ -6113,7 +6125,11 @@ Flow(.logical_datapath = r.lr._uuid, > _ -> "" > }, > var priority = if (lbvip.vip_port != 0) 120 else 110, > - var force_snat = if (force_snat_for_lb(r.lr)) "flags.force_snat_for_lb = 1; " else "", > + var force_snat = match (snat_for_lb(r.lr, lb)) { > + SkipSNAT -> "flags.skip_snat_for_lb = 1; ", > + ForceSNAT -> "flags.force_snat_for_lb = 1; ", > + _ -> "" > + }, > var actions = build_lb_vip_actions(lbvip, s_ROUTER_OUT_SNAT(), force_snat). > > > diff --git a/ovn-nb.xml b/ovn-nb.xml > index b0a4adffe..408c98090 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -1653,6 +1653,12 @@ > exactly one IPv4 and/or one IPv6 address on it, separated by a space > character. > </column> > + > + <column name="options" key="skip_snat"> > + If the load balancing rule is configured with <code>skip_snat</code> > + option, the force_snat_for_lb option configured for the router > + pipeline will not be applied for this load balancer. > + </column> > </group> > </table> > > diff --git a/ovs b/ovs > index ac85cdb38..ac09cbfcb 160000 > --- a/ovs > +++ b/ovs > @@ -1 +1 @@ > -Subproject commit ac85cdb38c1f33e7952bc4c0347d6c7873fb56a1 > +Subproject commit ac09cbfcb70ac6f443f039d5934448bd80f74493 > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index 96476497d..037ea98c5 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -2847,6 +2847,29 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl > table=1 (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) > ]) > > +check ovn-nbctl --wait=sb lb-add lb2 10.0.0.20:80 10.0.0.40:8080 > +check ovn-nbctl --wait=sb set load_balancer lb2 options:skip_snat=true > +check ovn-nbctl lr-lb-add lr0 lb2 > +check ovn-nbctl --wait=sb lb-del lb1 > +ovn-sbctl dump-flows lr0 > lr0flows > + > +AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl > + table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;) > + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-public" && ip4.dst == 172.168.0.100), action=(ct_snat;) > + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw0" && ip4.dst == 10.0.0.1), action=(ct_snat;) > + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip4.dst == 20.0.0.1), action=(ct_snat;) > + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip6.dst == bef0::1), action=(ct_snat;) > +]) > + > +AT_CHECK([grep "lr_in_dnat" lr0flows | grep skip_snat_for_lb | sort], [0], [dnl > + table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.20 && tcp && tcp.dst == 80), action=(flags.skip_snat_for_lb = 1; ct_dnat;) > + table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.20 && tcp && tcp.dst == 80), action=(flags.skip_snat_for_lb = 1; ct_lb(backends=10.0.0.40:8080);) > +]) > + > +AT_CHECK([grep "lr_out_snat" lr0flows | grep skip_snat_for_lb | sort], [0], [dnl > + table=1 (lr_out_snat ), priority=120 , match=(flags.skip_snat_for_lb == 1 && ip), action=(next;) > +]) > + > AT_CLEANUP > ]) > > @@ -2950,4 +2973,4 @@ wait_row_count FDB 0 > ovn-sbctl list FDB > > AT_CLEANUP > -]) > \ No newline at end of file > +]) >
diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h index 017176f98..d44b30b30 100644 --- a/include/ovn/logical-fields.h +++ b/include/ovn/logical-fields.h @@ -66,6 +66,7 @@ enum mff_log_flags_bits { MLF_LOOKUP_MAC_BIT = 6, MLF_LOOKUP_LB_HAIRPIN_BIT = 7, MLF_LOOKUP_FDB_BIT = 8, + MLF_SKIP_SNAT_FOR_LB_BIT = 9, }; /* MFF_LOG_FLAGS_REG flag assignments */ @@ -102,6 +103,10 @@ enum mff_log_flags { /* Indicate that the lookup in the fdb table was successful. */ MLF_LOOKUP_FDB = (1 << MLF_LOOKUP_FDB_BIT), + + /* Indicate that a packet must not SNAT in the gateway router when + * load-balancing has taken place. */ + MLF_SKIP_SNAT_FOR_LB = (1 << MLF_SKIP_SNAT_FOR_LB_BIT), }; /* OVN logical fields diff --git a/lib/logical-fields.c b/lib/logical-fields.c index 9d08b44c2..72853013e 100644 --- a/lib/logical-fields.c +++ b/lib/logical-fields.c @@ -121,6 +121,10 @@ ovn_init_symtab(struct shash *symtab) MLF_FORCE_SNAT_FOR_LB_BIT); expr_symtab_add_subfield(symtab, "flags.force_snat_for_lb", NULL, flags_str); + snprintf(flags_str, sizeof flags_str, "flags[%d]", + MLF_SKIP_SNAT_FOR_LB_BIT); + expr_symtab_add_subfield(symtab, "flags.skip_snat_for_lb", NULL, + flags_str); /* Connection tracking state. */ expr_symtab_add_field_scoped(symtab, "ct_mark", MFF_CT_MARK, NULL, false, diff --git a/northd/lrouter.dl b/northd/lrouter.dl index 36cedd2dc..312ceb9fc 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -355,8 +355,18 @@ function lb_force_snat_router_ip(lr_options: Map<string, string>): bool { lr_options.contains_key("chassis") } -function force_snat_for_lb(lr: nb::Logical_Router): bool { - not get_force_snat_ip(lr, "lb").is_empty() or lb_force_snat_router_ip(lr.options) +typedef LBForceSNAT = NoForceSNAT + | ForceSNAT + | SkipSNAT + +function snat_for_lb(lr: nb::Logical_Router, lb: Ref<nb::Load_Balancer>): LBForceSNAT { + if (lb.options.get_bool_def("skip_snat", false)) { + return SkipSNAT + }; + if (not get_force_snat_ip(lr, "lb").is_empty() or lb_force_snat_router_ip(lr.options)) { + return ForceSNAT + }; + return NoForceSNAT } /* For each router, collect the set of IPv4 and IPv6 addresses used for SNAT, diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml index a62f5c057..8197aa513 100644 --- a/northd/ovn-northd.8.xml +++ b/northd/ovn-northd.8.xml @@ -2754,7 +2754,11 @@ icmp6 { (and optional port numbers) to load balance to. If the router is configured to force SNAT any load-balanced packets, the above action will be replaced by <code>flags.force_snat_for_lb = 1; - ct_lb(<var>args</var>);</code>. If health check is enabled, then + ct_lb(<var>args</var>);</code>. + If the load balancing rule is configured with <code>skip_snat</code> + set to true, the above action will be replaced by + <code>flags.skip_snat_for_lb = 1; ct_lb(<var>args</var>);</code>. + If health check is enabled, then <var>args</var> will only contain those endpoints whose service monitor status entry in <code>OVN_Southbound</code> db is either <code>online</code> or empty. @@ -2771,6 +2775,9 @@ icmp6 { with an action of <code>ct_dnat;</code>. If the router is configured to force SNAT any load-balanced packets, the above action will be replaced by <code>flags.force_snat_for_lb = 1; ct_dnat;</code>. + If the load balancing rule is configured with <code>skip_snat</code> + set to true, the above action will be replaced by + <code>flags.skip_snat_for_lb = 1; ct_dnat;</code>. </li> <li> @@ -2785,6 +2792,9 @@ icmp6 { to force SNAT any load-balanced packets, the above action will be replaced by <code>flags.force_snat_for_lb = 1; ct_lb(<var>args</var>);</code>. + If the load balancing rule is configured with <code>skip_snat</code> + set to true, the above action will be replaced by + <code>flags.skip_snat_for_lb = 1; ct_lb(<var>args</var>);</code>. </li> <li> @@ -2797,6 +2807,9 @@ icmp6 { If the router is configured to force SNAT any load-balanced packets, the above action will be replaced by <code>flags.force_snat_for_lb = 1; ct_dnat;</code>. + If the load balancing rule is configured with <code>skip_snat</code> + set to true, the above action will be replaced by + <code>flags.skip_snat_for_lb = 1; ct_dnat;</code>. </li> <li> @@ -3829,6 +3842,15 @@ nd_ns { </p> </li> + <li> + <p> + If a load balancer configured to skip snat has been applied to + the Gateway router pipeline, a priority-120 flow matches + <code>flags.skip_snat_for_lb == 1 && ip</code> with an + action <code>next;</code>. + </p> + </li> + <li> <p> If the Gateway router in the OVN Northbound database has been diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 9839b8c4f..8de106c84 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -8573,10 +8573,16 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, return true; } +enum lb_snat_type { + NO_FORCE_SNAT, + FORCE_SNAT, + SKIP_SNAT, +}; + static void add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, struct ds *match, struct ds *actions, int priority, - bool force_snat_for_lb, struct ovn_lb_vip *lb_vip, + enum lb_snat_type snat_type, struct ovn_lb_vip *lb_vip, const char *proto, struct nbrec_load_balancer *lb, struct shash *meter_groups, struct sset *nat_entries) { @@ -8585,9 +8591,10 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, /* A match and actions for new connections. */ char *new_match = xasprintf("ct.new && %s", ds_cstr(match)); - if (force_snat_for_lb) { - char *new_actions = xasprintf("flags.force_snat_for_lb = 1; %s", - ds_cstr(actions)); + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { + char *new_actions = xasprintf("flags.%s_snat_for_lb = 1; %s", + snat_type == SKIP_SNAT ? "skip" : "force", + ds_cstr(actions)); ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, new_match, new_actions, &lb->header_); free(new_actions); @@ -8598,11 +8605,12 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, /* A match and actions for established connections. */ char *est_match = xasprintf("ct.est && %s", ds_cstr(match)); - if (force_snat_for_lb) { + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { + char *est_actions = xasprintf("flags.%s_snat_for_lb = 1; ct_dnat;", + snat_type == SKIP_SNAT ? "skip" : "force"); ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, - est_match, - "flags.force_snat_for_lb = 1; ct_dnat;", - &lb->header_); + est_match, est_actions, &lb->header_); + free(est_actions); } else { ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_DNAT, priority, est_match, "ct_dnat;", &lb->header_); @@ -8675,11 +8683,13 @@ add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od, ds_put_format(&undnat_match, ") && outport == %s && " "is_chassis_resident(%s)", od->l3dgw_port->json_key, od->l3redirect_port->json_key); - if (force_snat_for_lb) { + if (snat_type == FORCE_SNAT || snat_type == SKIP_SNAT) { + char *action = xasprintf("flags.%s_snat_for_lb = 1; ct_dnat;", + snat_type == SKIP_SNAT ? "skip" : "force"); ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120, - ds_cstr(&undnat_match), - "flags.force_snat_for_lb = 1; ct_dnat;", + ds_cstr(&undnat_match), action, &lb->header_); + free(action); } else { ovn_lflow_add_with_hint(lflows, od, S_ROUTER_OUT_UNDNAT, 120, ds_cstr(&undnat_match), "ct_dnat;", @@ -8706,6 +8716,12 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od, ovn_northd_lb_find(lbs, &nb_lb->header_.uuid); ovs_assert(lb); + bool lb_skip_snat = smap_get_bool(&nb_lb->options, "skip_snat", false); + if (lb_skip_snat) { + ovn_lflow_add(lflows, od, S_ROUTER_OUT_SNAT, 120, + "flags.skip_snat_for_lb == 1 && ip", "next;"); + } + for (size_t j = 0; j < lb->n_vips; j++) { struct ovn_lb_vip *lb_vip = &lb->vips[j]; struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[j]; @@ -8767,11 +8783,16 @@ build_lrouter_lb_flows(struct hmap *lflows, struct ovn_datapath *od, ds_put_format(match, " && is_chassis_resident(%s)", od->l3redirect_port->json_key); } - bool force_snat_for_lb = - lb_force_snat_ip || od->lb_force_snat_router_ip; + + enum lb_snat_type snat_type = NO_FORCE_SNAT; + if (lb_skip_snat) { + snat_type = SKIP_SNAT; + } else if (lb_force_snat_ip || od->lb_force_snat_router_ip) { + snat_type = FORCE_SNAT; + } add_router_lb_flow(lflows, od, match, actions, prio, - force_snat_for_lb, lb_vip, proto, - nb_lb, meter_groups, nat_entries); + snat_type, lb_vip, proto, nb_lb, + meter_groups, nat_entries); } } sset_destroy(&all_ips); diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 0063021e1..4c02e2d5a 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -5367,6 +5367,16 @@ for (&Router(.lr = lr)) { .external_ids = map_empty()) } +Flow(.logical_datapath = lr, + .stage = s_ROUTER_OUT_SNAT(), + .priority = 120, + .__match = "flags.skip_snat_for_lb == 1 && ip", + .actions = "next;", + .external_ids = stage_hint(lb._uuid)) :- + LogicalRouterLB(lr, lb), + lb.options.get_bool_def("skip_snat", false) + . + function lrouter_nat_is_stateless(nat: NAT): bool = { Some{"true"} == nat.nat.options.get("stateless") } @@ -6010,14 +6020,15 @@ for (RouterLBVIP( (Some{gwport}, true) -> " && is_chassis_resident(${redirect_port_name})", _ -> "" } in - var force_snat_for_lb = force_snat_for_lb(lr) in + var snat_for_lb = snat_for_lb(lr, lb) in { /* A match and actions for established connections. */ var est_match = "ct.est && " ++ __match in var actions = - match (force_snat_for_lb) { - true -> "flags.force_snat_for_lb = 1; ct_dnat;", - false -> "ct_dnat;" + match (snat_for_lb) { + SkipSNAT -> "flags.skip_snat_for_lb = 1; ct_dnat;", + ForceSNAT -> "flags.force_snat_for_lb = 1; ct_dnat;", + _ -> "ct_dnat;" } in Flow(.logical_datapath = lr._uuid, .stage = s_ROUTER_IN_DNAT(), @@ -6076,9 +6087,10 @@ for (RouterLBVIP( ") && outport == ${json_string_escape(gwport.name)} && " "is_chassis_resident(${redirect_port_name})" in var action = - match (force_snat_for_lb) { - true -> "flags.force_snat_for_lb = 1; ct_dnat;", - false -> "ct_dnat;" + match (snat_for_lb) { + SkipSNAT -> "flags.skip_snat_for_lb = 1; ct_dnat;", + ForceSNAT -> "flags.force_snat_for_lb = 1; ct_dnat;", + _ -> "ct_dnat;" } in Flow(.logical_datapath = lr._uuid, .stage = s_ROUTER_OUT_UNDNAT(), @@ -6113,7 +6125,11 @@ Flow(.logical_datapath = r.lr._uuid, _ -> "" }, var priority = if (lbvip.vip_port != 0) 120 else 110, - var force_snat = if (force_snat_for_lb(r.lr)) "flags.force_snat_for_lb = 1; " else "", + var force_snat = match (snat_for_lb(r.lr, lb)) { + SkipSNAT -> "flags.skip_snat_for_lb = 1; ", + ForceSNAT -> "flags.force_snat_for_lb = 1; ", + _ -> "" + }, var actions = build_lb_vip_actions(lbvip, s_ROUTER_OUT_SNAT(), force_snat). diff --git a/ovn-nb.xml b/ovn-nb.xml index b0a4adffe..408c98090 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -1653,6 +1653,12 @@ exactly one IPv4 and/or one IPv6 address on it, separated by a space character. </column> + + <column name="options" key="skip_snat"> + If the load balancing rule is configured with <code>skip_snat</code> + option, the force_snat_for_lb option configured for the router + pipeline will not be applied for this load balancer. + </column> </group> </table> diff --git a/ovs b/ovs index ac85cdb38..ac09cbfcb 160000 --- a/ovs +++ b/ovs @@ -1 +1 @@ -Subproject commit ac85cdb38c1f33e7952bc4c0347d6c7873fb56a1 +Subproject commit ac09cbfcb70ac6f443f039d5934448bd80f74493 diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 96476497d..037ea98c5 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -2847,6 +2847,29 @@ AT_CHECK([grep "lr_out_snat" lr0flows | sort], [0], [dnl table=1 (lr_out_snat ), priority=120 , match=(nd_ns), action=(next;) ]) +check ovn-nbctl --wait=sb lb-add lb2 10.0.0.20:80 10.0.0.40:8080 +check ovn-nbctl --wait=sb set load_balancer lb2 options:skip_snat=true +check ovn-nbctl lr-lb-add lr0 lb2 +check ovn-nbctl --wait=sb lb-del lb1 +ovn-sbctl dump-flows lr0 > lr0flows + +AT_CHECK([grep "lr_in_unsnat" lr0flows | sort], [0], [dnl + table=5 (lr_in_unsnat ), priority=0 , match=(1), action=(next;) + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-public" && ip4.dst == 172.168.0.100), action=(ct_snat;) + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw0" && ip4.dst == 10.0.0.1), action=(ct_snat;) + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip4.dst == 20.0.0.1), action=(ct_snat;) + table=5 (lr_in_unsnat ), priority=110 , match=(inport == "lr0-sw1" && ip6.dst == bef0::1), action=(ct_snat;) +]) + +AT_CHECK([grep "lr_in_dnat" lr0flows | grep skip_snat_for_lb | sort], [0], [dnl + table=6 (lr_in_dnat ), priority=120 , match=(ct.est && ip && ip4.dst == 10.0.0.20 && tcp && tcp.dst == 80), action=(flags.skip_snat_for_lb = 1; ct_dnat;) + table=6 (lr_in_dnat ), priority=120 , match=(ct.new && ip && ip4.dst == 10.0.0.20 && tcp && tcp.dst == 80), action=(flags.skip_snat_for_lb = 1; ct_lb(backends=10.0.0.40:8080);) +]) + +AT_CHECK([grep "lr_out_snat" lr0flows | grep skip_snat_for_lb | sort], [0], [dnl + table=1 (lr_out_snat ), priority=120 , match=(flags.skip_snat_for_lb == 1 && ip), action=(next;) +]) + AT_CLEANUP ]) @@ -2950,4 +2973,4 @@ wait_row_count FDB 0 ovn-sbctl list FDB AT_CLEANUP -]) \ No newline at end of file +])