On Thu, Dec 10, 2020 at 3:18 AM Mark Michelson <[email protected]> wrote: > > Acked-by: Mark Michelson <[email protected]>
Thanks Lorenzo for v2 and Mark. I applied this patch to master. Numan > > On 12/9/20 6:25 AM, Lorenzo Bianconi wrote: > > Introduce the capability to create a load balancer with no backends and > > with --reject option in order to send a TCP reset segment (for tcp) or > > an ICMP port unreachable packet (for all other kind of traffic) whenever > > an incoming packet is received for this load-balancer. > > > > Tested-by: Antonio Ojea <[email protected]> > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > --- > > Changes since v2: > > - introduce ovn-northd.at self-test > > > > Changes since v1: > > - add reject action for health_check empty lb > > - rename build_lb_vip_ct_lb_actions in build_lb_vip_actions > > - improve documentation > > --- > > lib/lb.c | 2 ++ > > lib/lb.h | 1 + > > northd/ovn-northd.8.xml | 19 +++++++++++++++++ > > northd/ovn-northd.c | 44 ++++++++++++++++++++++++++------------- > > ovn-nb.ovsschema | 9 ++++++-- > > ovn-nb.xml | 10 +++++++++ > > tests/ovn-northd.at | 25 ++++++++++++++++++++++ > > tests/system-ovn.at | 28 ++++++++++++++++++++++++- > > utilities/ovn-nbctl.8.xml | 11 +++++++++- > > utilities/ovn-nbctl.c | 7 ++++++- > > 10 files changed, 136 insertions(+), 20 deletions(-) > > > > diff --git a/lib/lb.c b/lib/lb.c > > index a90042e58..2517c02ef 100644 > > --- a/lib/lb.c > > +++ b/lib/lb.c > > @@ -189,6 +189,8 @@ ovn_northd_lb_create(const struct nbrec_load_balancer > > *nbrec_lb, > > struct ovn_lb_vip *lb_vip = &lb->vips[n_vips]; > > struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[n_vips]; > > > > + lb_vip->empty_backend_rej = smap_get_bool(&nbrec_lb->options, > > + "reject", false); > > if (!ovn_lb_vip_init(lb_vip, node->key, node->value)) { > > continue; > > } > > diff --git a/lib/lb.h b/lib/lb.h > > index 6644ad0d8..42c580bd1 100644 > > --- a/lib/lb.h > > +++ b/lib/lb.h > > @@ -49,6 +49,7 @@ struct ovn_lb_vip { > > > > struct ovn_lb_backend *backends; > > size_t n_backends; > > + bool empty_backend_rej; > > }; > > > > struct ovn_lb_backend { > > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml > > index 294402de3..8bbe577b6 100644 > > --- a/northd/ovn-northd.8.xml > > +++ b/northd/ovn-northd.8.xml > > @@ -700,6 +700,16 @@ > > ct_lb(<var>args</var>)</code>, where <var>args</var> contains > > comma > > separated IP addresses of the same address family as > > <var>VIP</var>. > > </li> > > + > > + <li> > > + If the load balancer is created with <code>--reject</code> option > > and > > + it has no active backends, a TCP reset segment (for tcp) or an ICMP > > + port unreachable packet (for all other kind of traffic) will be > > sent > > + whenever an incoming packet is received for this load-balancer. > > + Please note using <code>--reject</code> option will disable > > + empty_lb SB controller event for this load balancer. > > + </li> > > + > > <li> > > A priority-100 flow commits packets to connection tracker using > > <code>ct_commit; next;</code> action based on a hint provided by > > @@ -2592,6 +2602,15 @@ icmp6 { > > packets, the above action will be replaced by > > <code>flags.force_snat_for_lb = 1; ct_dnat;</code>. > > </li> > > + > > + <li> > > + If the load balancer is created with <code>--reject</code> option > > and > > + it has no active backends, a TCP reset segment (for tcp) or an ICMP > > + port unreachable packet (for all other kind of traffic) will be > > sent > > + whenever an incoming packet is received for this load-balancer. > > + Please note using <code>--reject</code> option will disable > > + empty_lb SB controller event for this load balancer. > > + </li> > > </ul> > > > > <p>Ingress Table 6: DNAT on Gateway Routers</p> > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 957242367..6b83f8e9e 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -3436,12 +3436,12 @@ ovn_lb_svc_create(struct northd_context *ctx, > > struct ovn_northd_lb *lb, > > } > > > > static > > -void build_lb_vip_ct_lb_actions(struct ovn_lb_vip *lb_vip, > > - struct ovn_northd_lb_vip *lb_vip_nb, > > - struct ds *action, > > - char *selection_fields) > > +void build_lb_vip_actions(struct ovn_lb_vip *lb_vip, > > + struct ovn_northd_lb_vip *lb_vip_nb, > > + struct ds *action, char *selection_fields, > > + bool ls_dp) > > { > > - bool skip_hash_fields = false; > > + bool skip_hash_fields = false, reject = false; > > > > if (lb_vip_nb->lb_health_check) { > > ds_put_cstr(action, "ct_lb(backends="); > > @@ -3463,18 +3463,30 @@ void build_lb_vip_ct_lb_actions(struct ovn_lb_vip > > *lb_vip, > > } > > > > if (!n_active_backends) { > > - skip_hash_fields = true; > > - ds_clear(action); > > - ds_put_cstr(action, "drop;"); > > + if (!lb_vip->empty_backend_rej) { > > + ds_clear(action); > > + ds_put_cstr(action, "drop;"); > > + skip_hash_fields = true; > > + } else { > > + reject = true; > > + } > > } else { > > ds_chomp(action, ','); > > ds_put_cstr(action, ");"); > > } > > + } else if (lb_vip->empty_backend_rej && !lb_vip->n_backends) { > > + reject = true; > > } else { > > ds_put_format(action, "ct_lb(backends=%s);", > > lb_vip_nb->backend_ips); > > } > > > > - if (!skip_hash_fields && selection_fields && selection_fields[0]) { > > + if (reject) { > > + int stage = ls_dp ? ovn_stage_get_table(S_SWITCH_OUT_QOS_MARK) > > + : ovn_stage_get_table(S_ROUTER_OUT_SNAT); > > + ds_clear(action); > > + ds_put_format(action, "reg0 = 0; reject { outport <-> inport; " > > + "next(pipeline=egress,table=%d);};", stage); > > + } else if (!skip_hash_fields && selection_fields && > > selection_fields[0]) { > > ds_chomp(action, ';'); > > ds_chomp(action, ')'); > > ds_put_format(action, "; hash_fields=\"%s\");", selection_fields); > > @@ -5078,7 +5090,8 @@ build_empty_lb_event_flow(struct ovn_datapath *od, > > struct hmap *lflows, > > struct nbrec_load_balancer *lb, > > int pl, struct shash *meter_groups) > > { > > - if (!controller_event_en || lb_vip->n_backends) { > > + if (!controller_event_en || lb_vip->n_backends || > > + lb_vip->empty_backend_rej) { > > return; > > } > > > > @@ -5968,8 +5981,8 @@ build_lb_rules(struct ovn_datapath *od, struct hmap > > *lflows, > > > > /* New connections in Ingress table. */ > > struct ds action = DS_EMPTY_INITIALIZER; > > - build_lb_vip_ct_lb_actions(lb_vip, lb_vip_nb, &action, > > - lb->selection_fields); > > + build_lb_vip_actions(lb_vip, lb_vip_nb, &action, > > + lb->selection_fields, true); > > > > struct ds match = DS_EMPTY_INITIALIZER; > > ds_put_format(&match, "ct.new && %s.dst == %s", ip_match, > > @@ -9679,8 +9692,8 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > struct ovn_lb_vip *lb_vip = &lb->vips[j]; > > struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[j]; > > ds_clear(&actions); > > - build_lb_vip_ct_lb_actions(lb_vip, lb_vip_nb, &actions, > > - lb->selection_fields); > > + build_lb_vip_actions(lb_vip, lb_vip_nb, &actions, > > + lb->selection_fields, false); > > > > if (!sset_contains(&all_ips, lb_vip->vip_str)) { > > sset_add(&all_ips, lb_vip->vip_str); > > @@ -9731,7 +9744,8 @@ build_lrouter_flows(struct hmap *datapaths, struct > > hmap *ports, > > prio = 120; > > } > > > > - if (od->l3redirect_port) { > > + if (od->l3redirect_port && > > + (lb_vip->n_backends || !lb_vip->empty_backend_rej)) { > > ds_put_format(&match, " && is_chassis_resident(%s)", > > od->l3redirect_port->json_key); > > } > > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > > index 269e3a888..af77dd138 100644 > > --- a/ovn-nb.ovsschema > > +++ b/ovn-nb.ovsschema > > @@ -1,7 +1,7 @@ > > { > > "name": "OVN_Northbound", > > - "version": "5.28.0", > > - "cksum": "610359755 26847", > > + "version": "5.29.0", > > + "cksum": "328602112 27064", > > "tables": { > > "NB_Global": { > > "columns": { > > @@ -188,6 +188,11 @@ > > ["eth_src", "eth_dst", "ip_src", "ip_dst", > > "tp_src", "tp_dst"]]}, > > "min": 0, "max": "unlimited"}}, > > + "options": { > > + "type": {"key": "string", > > + "value": "string", > > + "min": 0, > > + "max": "unlimited"}}, > > "external_ids": { > > "type": {"key": "string", "value": "string", > > "min": 0, "max": "unlimited"}}}, > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index c9ab25ceb..e7a8d6833 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -1635,6 +1635,16 @@ > > See <em>External IDs</em> at the beginning of this document. > > </column> > > </group> > > + <group title="Load_Balancer options"> > > + <column name="options" key="reject" type='{"type": "boolean"}'> > > + If the load balancer is created with <code>--reject</code> option > > and > > + it has no active backends, a TCP reset segment (for tcp) or an ICMP > > + port unreachable packet (for all other kind of traffic) will be > > sent > > + whenever an incoming packet is received for this load-balancer. > > + Please note using <code>--reject</code> option will disable > > empty_lb > > + SB controller event for this load balancer. > > + </column> > > + </group> > > </table> > > > > <table name="Load_Balancer_Health_Check" title="load balancer"> > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > > index 90ca0a4db..50a4cae76 100644 > > --- a/tests/ovn-northd.at > > +++ b/tests/ovn-northd.at > > @@ -1233,6 +1233,31 @@ wait_row_count Service_Monitor 2 > > ovn-nbctl --wait=sb lb-del lb2 > > wait_row_count Service_Monitor 0 > > > > +check ovn-nbctl --reject lb-add lb3 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80 > > +check ovn-nbctl --wait=sb set load_balancer lb3 > > ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2 > > +check ovn-nbctl --wait=sb set load_balancer lb3 > > ip_port_mappings:20.0.0.3=sw1-p1:20.0.0.2 > > +wait_row_count Service_Monitor 0 > > + > > +check ovn-nbctl --wait=sb ls-lb-add sw0 lb3 > > +AT_CHECK([ovn-nbctl --wait=sb -- --id=@hc create \ > > +Load_Balancer_Health_Check vip="10.0.0.10\:80" -- add Load_Balancer lb3 \ > > +health_check @hc | uuidfilt], [0], [<0> > > +]) > > +wait_row_count Service_Monitor 2 > > + > > +# Set the service monitor for sw0-p1 and sw1-p1 to online > > +sm_sw0_p1=$(fetch_column Service_Monitor _uuid logical_port=sw0-p1) > > +sm_sw1_p1=$(fetch_column Service_Monitor _uuid logical_port=sw1-p1) > > + > > +ovn-sbctl set service_monitor $sm_sw0_p1 status=offline > > +ovn-sbctl set service_monitor $sm_sw1_p1 status=offline > > + > > +AT_CAPTURE_FILE([sbflows12]) > > +OVS_WAIT_FOR_OUTPUT( > > + [ovn-sbctl dump-flows sw0 | tee sbflows12 | grep "ip4.dst == 10.0.0.10 > > && tcp.dst == 80" | grep priority=120 | sed 's/table=..//'], [0], [dnl > > + (ls_in_stateful ), priority=120 , match=(ct.new && ip4.dst == > > 10.0.0.10 && tcp.dst == 80), action=(reg0 = 0; reject { outport <-> inport; > > next(pipeline=egress,table=6);};) > > +]) > > + > > AT_CLEANUP > > > > AT_SETUP([ovn -- Load balancer VIP in NAT entries]) > > diff --git a/tests/system-ovn.at b/tests/system-ovn.at > > index d59f7c97e..1e73001ab 100644 > > --- a/tests/system-ovn.at > > +++ b/tests/system-ovn.at > > @@ -1574,6 +1574,18 @@ OVS_WAIT_UNTIL([ > > grep "selection_method=hash,fields(ip_src,ip_dst,sctp_src,sctp_dst)" > > -c) -eq 2 > > ]) > > > > +ovn-nbctl --reject lb-add lb3 30.0.0.10:80 "" > > +ovn-nbctl ls-lb-add foo lb3 > > +# Filter reset segments > > +NS_CHECK_EXEC([foo1], [tcpdump -c 1 -neei foo1 ip[[33:1]]=0x14 > rst.pcap > > &]) > > +sleep 1 > > +NS_CHECK_EXEC([foo1], [wget -q 30.0.0.10],[4]) > > + > > +OVS_WAIT_UNTIL([ > > + n_reset=$(cat rst.pcap | wc -l) > > + test "${n_reset}" = "1" > > +]) > > + > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > > > as ovn-sb > > @@ -4151,7 +4163,7 @@ ovn-nbctl lsp-set-type sw1-lr0 router > > ovn-nbctl lsp-set-addresses sw1-lr0 router > > ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1 > > > > -ovn-nbctl lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80 > > +ovn-nbctl --reject lb-add lb1 10.0.0.10:80 10.0.0.3:80,20.0.0.3:80 > > > > ovn-nbctl --wait=sb set load_balancer . > > ip_port_mappings:10.0.0.3=sw0-p1:10.0.0.2 > > ovn-nbctl --wait=sb set load_balancer . > > ip_port_mappings:20.0.0.3=sw1-p1:20.0.0.2 > > @@ -4266,6 +4278,20 @@ ovn-sbctl list service_monitor > > OVS_WAIT_UNTIL([test 2 = `ovn-sbctl --bare --columns status find \ > > service_monitor protocol=udp | sed '/^$/d' | grep offline | wc -l`]) > > > > +# Stop webserer in sw1-p1 > > +pid_file=$(cat l7_pid_file) > > +NS_CHECK_EXEC([sw1-p1], [kill $(cat $pid_file)]) > > + > > +NS_CHECK_EXEC([sw0-p2], [tcpdump -c 1 -neei sw0-p2 ip[[33:1]]=0x14 > > > rst.pcap &]) > > +OVS_WAIT_UNTIL([test 2 = `ovn-sbctl --bare --columns status find \ > > +service_monitor protocol=tcp | sed '/^$/d' | grep offline | wc -l`]) > > +NS_CHECK_EXEC([sw0-p2], [wget 10.0.0.10 -v -o wget$i.log],[4]) > > + > > +OVS_WAIT_UNTIL([ > > + n_reset=$(cat rst.pcap | wc -l) > > + test "${n_reset}" = "1" > > +]) > > + > > OVS_APP_EXIT_AND_WAIT([ovn-controller]) > > > > as ovn-sb > > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > > index 59302296b..e5a35f307 100644 > > --- a/utilities/ovn-nbctl.8.xml > > +++ b/utilities/ovn-nbctl.8.xml > > @@ -903,7 +903,7 @@ > > > > <h1>Load Balancer Commands</h1> > > <dl> > > - <dt>[<code>--may-exist</code> | <code>--add-duplicate</code>] > > <code>lb-add</code> <var>lb</var> <var>vip</var> <var>ips</var> > > [<var>protocol</var>]</dt> > > + <dt>[<code>--may-exist</code> | <code>--add-duplicate</code> | > > <code>--reject</code>] <code>lb-add</code> <var>lb</var> <var>vip</var> > > <var>ips</var> [<var>protocol</var>]</dt> > > <dd> > > <p> > > Creates a new load balancer named <var>lb</var> with the provided > > @@ -936,6 +936,15 @@ > > creates a new load balancer with a duplicate name. > > </p> > > > > + <p> > > + If the load balancer is created with <code>--reject</code> option > > and > > + it has no active backends, a TCP reset segment (for tcp) or an > > ICMP > > + port unreachable packet (for all other kind of traffic) will be > > sent > > + whenever an incoming packet is received for this load-balancer. > > + Please note using <code>--reject</code> option will disable > > + empty_lb SB controller event for this load balancer. > > + </p> > > + > > <p> > > The following example adds a load balancer. > > </p> > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > > index d19e1b6c6..3a95f6b1f 100644 > > --- a/utilities/ovn-nbctl.c > > +++ b/utilities/ovn-nbctl.c > > @@ -2821,6 +2821,7 @@ nbctl_lb_add(struct ctl_context *ctx) > > > > bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL; > > bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != > > NULL; > > + bool empty_backend_rej = shash_find(&ctx->options, "--reject") != NULL; > > > > const char *lb_proto; > > bool is_update_proto = false; > > @@ -2934,6 +2935,10 @@ nbctl_lb_add(struct ctl_context *ctx) > > smap_add(CONST_CAST(struct smap *, &lb->vips), > > lb_vip_normalized, ds_cstr(&lb_ips_new)); > > nbrec_load_balancer_set_vips(lb, &lb->vips); > > + if (empty_backend_rej) { > > + const struct smap options = SMAP_CONST1(&options, "reject", > > "true"); > > + nbrec_load_balancer_set_options(lb, &options); > > + } > > out: > > ds_destroy(&lb_ips_new); > > > > @@ -6588,7 +6593,7 @@ static const struct ctl_command_syntax > > nbctl_commands[] = { > > nbctl_lr_nat_set_ext_ips, NULL, "--is-exempted", RW}, > > /* load balancer commands. */ > > { "lb-add", 3, 4, "LB VIP[:PORT] IP[:PORT]... [PROTOCOL]", NULL, > > - nbctl_lb_add, NULL, "--may-exist,--add-duplicate", RW }, > > + nbctl_lb_add, NULL, "--may-exist,--add-duplicate,--reject", RW }, > > { "lb-del", 1, 2, "LB [VIP]", NULL, nbctl_lb_del, NULL, > > "--if-exists", RW }, > > { "lb-list", 0, 1, "[LB]", NULL, nbctl_lb_list, NULL, "", RO }, > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
