[ovs-dev] [PATCH v3 ovn] northd: add reject action for lb with no backends

Lorenzo Bianconi lorenzo.bianconi at redhat.com
Wed Dec 9 11:25:31 UTC 2020


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 <aojeagar at redhat.com>
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
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 },
-- 
2.29.2



More information about the dev mailing list