From f21c1b7a467a691847b5552d4570af706fcc5bb0 Mon Sep 17 00:00:00 2001

Message-Id: <f21c1b7a467a691847b5552d4570af706fcc5bb0.1610458802.git.lorenzo.bianconi@redhat.com>

From: Anton Ivanov <anton.ivanov@cambridgegreys.com>

Date: Tue, 5 Jan 2021 17:49:28 +0000

Subject: [PATCH 01/16] ovn-northd: Move lswitch ARP/ND Responder to functions.

Move arp/nd responder lflow processing to per-iterable functions.

Signed-off-by: Anton Ivanov <anton.ivanov@cambridgegreys.com>

Signed-off-by: Numan Siddique <numans@ovn.org>

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>

---

 northd/ovn-northd.c | 496 +++++++++++++++++++++++---------------------

 1 file changed, 260 insertions(+), 236 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c

index b377dffa1..d17cc55ac 100644

--- a/northd/ovn-northd.c

+++ b/northd/ovn-northd.c

@@ -6770,7 +6770,7 @@ is_vlan_transparent(const struct ovn_datapath *od)

 static void

 build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,

                     struct hmap *lflows, struct hmap *mcgroups,

-                    struct hmap *igmp_groups, struct hmap *lbs)

+                    struct hmap *igmp_groups)

 {

     /* This flow table structure is documented in ovn-northd(8), so please

      * update ovn-northd.8.xml if you change anything. */

@@ -6778,240 +6778,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,

     struct ds match = DS_EMPTY_INITIALIZER;

     struct ds actions = DS_EMPTY_INITIALIZER;

     struct ovn_datapath *od;

-

-    /* Ingress table 13: ARP/ND responder, skip requests coming from localnet

-     * and vtep ports. (priority 100); see ovn-northd.8.xml for the

-     * rationale. */

     struct ovn_port *op;

-    HMAP_FOR_EACH (op, key_node, ports) {

-        if (!op->nbsp) {

-            continue;

-        }

-

-        if ((!strcmp(op->nbsp->type, "localnet")) ||

-            (!strcmp(op->nbsp->type, "vtep"))) {

-            ds_clear(&match);

-            ds_put_format(&match, "inport == %s", op->json_key);

-            ovn_lflow_add_with_hint(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP,

-                                    100, ds_cstr(&match), "next;",

-                                    &op->nbsp->header_);

-        }

-    }

-

-    /* Ingress table 13: ARP/ND responder, reply for known IPs.

-     * (priority 50). */

-    HMAP_FOR_EACH (op, key_node, ports) {

-        if (!op->nbsp) {

-            continue;

-        }

-

-        if (!strcmp(op->nbsp->type, "virtual")) {

-            /* Handle

-             *  - GARPs for virtual ip which belongs to a logical port

-             *    of type 'virtual' and bind that port.

-             *

-             *  - ARP reply from the virtual ip which belongs to a logical

-             *    port of type 'virtual' and bind that port.

-             * */

-            ovs_be32 ip;

-            const char *virtual_ip = smap_get(&op->nbsp->options,

-                                              "virtual-ip");

-            const char *virtual_parents = smap_get(&op->nbsp->options,

-                                                   "virtual-parents");

-            if (!virtual_ip || !virtual_parents ||

-                !ip_parse(virtual_ip, &ip)) {

-                continue;

-            }

-

-            char *tokstr = xstrdup(virtual_parents);

-            char *save_ptr = NULL;

-            char *vparent;

-            for (vparent = strtok_r(tokstr, ",", &save_ptr); vparent != NULL;

-                 vparent = strtok_r(NULL, ",", &save_ptr)) {

-                struct ovn_port *vp = ovn_port_find(ports, vparent);

-                if (!vp || vp->od != op->od) {

-                    /* vparent name should be valid and it should belong

-                     * to the same logical switch. */

-                    continue;

-                }

-

-                ds_clear(&match);

-                ds_put_format(&match, "inport == \"%s\" && "

-                              "((arp.op == 1 && arp.spa == %s && "

-                              "arp.tpa == %s) || (arp.op == 2 && "

-                              "arp.spa == %s))",

-                              vparent, virtual_ip, virtual_ip,

-                              virtual_ip);

-                ds_clear(&actions);

-                ds_put_format(&actions,

-                    "bind_vport(%s, inport); "

-                    "next;",

-                    op->json_key);

-                ovn_lflow_add_with_hint(lflows, op->od,

-                                        S_SWITCH_IN_ARP_ND_RSP, 100,

-                                        ds_cstr(&match), ds_cstr(&actions),

-                                        &vp->nbsp->header_);

-            }

-

-            free(tokstr);

-        } else {

-            /*

-             * Add ARP/ND reply flows if either the

-             *  - port is up and it doesn't have 'unknown' address defined or

-             *  - port type is router or

-             *  - port type is localport

-             */

-            if (check_lsp_is_up &&

-                !lsp_is_up(op->nbsp) && !lsp_is_router(op->nbsp) &&

-                strcmp(op->nbsp->type, "localport")) {

-                continue;

-            }

-

-            if (lsp_is_external(op->nbsp) || op->has_unknown) {

-                continue;

-            }

-

-            for (size_t i = 0; i < op->n_lsp_addrs; i++) {

-                for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {

-                    ds_clear(&match);

-                    ds_put_format(&match, "arp.tpa == %s && arp.op == 1",

-                                op->lsp_addrs[i].ipv4_addrs[j].addr_s);

-                    ds_clear(&actions);

-                    ds_put_format(&actions,

-                        "eth.dst = eth.src; "

-                        "eth.src = %s; "

-                        "arp.op = 2; /* ARP reply */ "

-                        "arp.tha = arp.sha; "

-                        "arp.sha = %s; "

-                        "arp.tpa = arp.spa; "

-                        "arp.spa = %s; "

-                        "outport = inport; "

-                        "flags.loopback = 1; "

-                        "output;",

-                        op->lsp_addrs[i].ea_s, op->lsp_addrs[i].ea_s,

-                        op->lsp_addrs[i].ipv4_addrs[j].addr_s);

-                    ovn_lflow_add_with_hint(lflows, op->od,

-                                            S_SWITCH_IN_ARP_ND_RSP, 50,

-                                            ds_cstr(&match),

-                                            ds_cstr(&actions),

-                                            &op->nbsp->header_);

-

-                    /* Do not reply to an ARP request from the port that owns

-                     * the address (otherwise a DHCP client that ARPs to check

-                     * for a duplicate address will fail).  Instead, forward

-                     * it the usual way.

-                     *

-                     * (Another alternative would be to simply drop the packet.

-                     * If everything is working as it is configured, then this

-                     * would produce equivalent results, since no one should

-                     * reply to the request.  But ARPing for one's own IP

-                     * address is intended to detect situations where the

-                     * network is not working as configured, so dropping the

-                     * request would frustrate that intent.) */

-                    ds_put_format(&match, " && inport == %s", op->json_key);

-                    ovn_lflow_add_with_hint(lflows, op->od,

-                                            S_SWITCH_IN_ARP_ND_RSP, 100,

-                                            ds_cstr(&match), "next;",

-                                            &op->nbsp->header_);

-                }

-

-                /* For ND solicitations, we need to listen for both the

-                 * unicast IPv6 address and its all-nodes multicast address,

-                 * but always respond with the unicast IPv6 address. */

-                for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) {

-                    ds_clear(&match);

-                    ds_put_format(&match,

-                            "nd_ns && ip6.dst == {%s, %s} && nd.target == %s",

-                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

-                            op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s,

-                            op->lsp_addrs[i].ipv6_addrs[j].addr_s);

-

-                    ds_clear(&actions);

-                    ds_put_format(&actions,

-                            "%s { "

-                            "eth.src = %s; "

-                            "ip6.src = %s; "

-                            "nd.target = %s; "

-                            "nd.tll = %s; "

-                            "outport = inport; "

-                            "flags.loopback = 1; "

-                            "output; "

-                            "};",

-                            lsp_is_router(op->nbsp) ? "nd_na_router" : "nd_na",

-                            op->lsp_addrs[i].ea_s,

-                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

-                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

-                            op->lsp_addrs[i].ea_s);

-                    ovn_lflow_add_with_hint(lflows, op->od,

-                                            S_SWITCH_IN_ARP_ND_RSP, 50,

-                                            ds_cstr(&match),

-                                            ds_cstr(&actions),

-                                            &op->nbsp->header_);

-

-                    /* Do not reply to a solicitation from the port that owns

-                     * the address (otherwise DAD detection will fail). */

-                    ds_put_format(&match, " && inport == %s", op->json_key);

-                    ovn_lflow_add_with_hint(lflows, op->od,

-                                            S_SWITCH_IN_ARP_ND_RSP, 100,

-                                            ds_cstr(&match), "next;",

-                                            &op->nbsp->header_);

-                }

-            }

-        }

-    }

-

-    /* Ingress table 13: ARP/ND responder, by default goto next.

-     * (priority 0)*/

-    HMAP_FOR_EACH (od, key_node, datapaths) {

-        if (!od->nbs) {

-            continue;

-        }

-

-        ovn_lflow_add(lflows, od, S_SWITCH_IN_ARP_ND_RSP, 0, "1", "next;");

-    }

-

-    /* Ingress table 13: ARP/ND responder for service monitor source ip.

-     * (priority 110)*/

-    struct ovn_northd_lb *lb;

-    HMAP_FOR_EACH (lb, hmap_node, lbs) {

-        for (size_t i = 0; i < lb->n_vips; i++) {

-            struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[i];

-            if (!lb_vip_nb->lb_health_check) {

-                continue;

-            }

-

-            for (size_t j = 0; j < lb_vip_nb->n_backends; j++) {

-                struct ovn_northd_lb_backend *backend_nb =

-                    &lb_vip_nb->backends_nb[j];

-                if (!backend_nb->op || !backend_nb->svc_mon_src_ip) {

-                    continue;

-                }

-

-                ds_clear(&match);

-                ds_put_format(&match, "arp.tpa == %s && arp.op == 1",

-                              backend_nb->svc_mon_src_ip);

-                ds_clear(&actions);

-                ds_put_format(&actions,

-                    "eth.dst = eth.src; "

-                    "eth.src = %s; "

-                    "arp.op = 2; /* ARP reply */ "

-                    "arp.tha = arp.sha; "

-                    "arp.sha = %s; "

-                    "arp.tpa = arp.spa; "

-                    "arp.spa = %s; "

-                    "outport = inport; "

-                    "flags.loopback = 1; "

-                    "output;",

-                    svc_monitor_mac, svc_monitor_mac,

-                    backend_nb->svc_mon_src_ip);

-                ovn_lflow_add_with_hint(lflows,

-                                        backend_nb->op->od,

-                                        S_SWITCH_IN_ARP_ND_RSP, 110,

-                                        ds_cstr(&match), ds_cstr(&actions),

-                                        &lb->nlb->header_);

-            }

-        }

-    }

     /* Logical switch ingress table 14 and 15: DHCP options and response

@@ -7471,6 +7238,251 @@ build_lswitch_lflows_admission_control(struct ovn_datapath *od,

     }

 }

+/* Ingress table 13: ARP/ND responder, skip requests coming from localnet

+ * and vtep ports. (priority 100); see ovn-northd.8.xml for the

+ * rationale. */

+

+static void

+build_lswitch_arp_nd_responder_skip_local(struct ovn_port *op,

+                                          struct hmap *lflows,

+                                          struct ds *match)

+{

+    if (op->nbsp) {

+        if ((!strcmp(op->nbsp->type, "localnet")) ||

+            (!strcmp(op->nbsp->type, "vtep"))) {

+            ds_clear(match);

+            ds_put_format(match, "inport == %s", op->json_key);

+            ovn_lflow_add_with_hint(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP,

+                                    100, ds_cstr(match), "next;",

+                                    &op->nbsp->header_);

+        }

+    }

+}

+

+/* Ingress table 13: ARP/ND responder, reply for known IPs.

+ * (priority 50). */

+static void

+build_lswitch_arp_nd_responder_known_ips(struct ovn_port *op,

+                                         struct hmap *lflows,

+                                         struct hmap *ports,

+                                         struct ds *actions,

+                                         struct ds *match)

+{

+    if (op->nbsp) {

+        if (!strcmp(op->nbsp->type, "virtual")) {

+            /* Handle

+             *  - GARPs for virtual ip which belongs to a logical port

+             *    of type 'virtual' and bind that port.

+             *

+             *  - ARP reply from the virtual ip which belongs to a logical

+             *    port of type 'virtual' and bind that port.

+             * */

+            ovs_be32 ip;

+            const char *virtual_ip = smap_get(&op->nbsp->options,

+                                              "virtual-ip");

+            const char *virtual_parents = smap_get(&op->nbsp->options,

+                                                   "virtual-parents");

+            if (!virtual_ip || !virtual_parents ||

+                !ip_parse(virtual_ip, &ip)) {

+                return;

+            }

+

+            char *tokstr = xstrdup(virtual_parents);

+            char *save_ptr = NULL;

+            char *vparent;

+            for (vparent = strtok_r(tokstr, ",", &save_ptr); vparent != NULL;

+                 vparent = strtok_r(NULL, ",", &save_ptr)) {

+                struct ovn_port *vp = ovn_port_find(ports, vparent);

+                if (!vp || vp->od != op->od) {

+                    /* vparent name should be valid and it should belong

+                     * to the same logical switch. */

+                    continue;

+                }

+

+                ds_clear(match);

+                ds_put_format(match, "inport == \"%s\" && "

+                              "((arp.op == 1 && arp.spa == %s && "

+                              "arp.tpa == %s) || (arp.op == 2 && "

+                              "arp.spa == %s))",

+                              vparent, virtual_ip, virtual_ip,

+                              virtual_ip);

+                ds_clear(actions);

+                ds_put_format(actions,

+                    "bind_vport(%s, inport); "

+                    "next;",

+                    op->json_key);

+                ovn_lflow_add_with_hint(lflows, op->od,

+                                        S_SWITCH_IN_ARP_ND_RSP, 100,

+                                        ds_cstr(match), ds_cstr(actions),

+                                        &vp->nbsp->header_);

+            }

+

+            free(tokstr);

+        } else {

+            /*

+             * Add ARP/ND reply flows if either the

+             *  - port is up and it doesn't have 'unknown' address defined or

+             *  - port type is router or

+             *  - port type is localport

+             */

+            if (check_lsp_is_up &&

+                !lsp_is_up(op->nbsp) && !lsp_is_router(op->nbsp) &&

+                strcmp(op->nbsp->type, "localport")) {

+                return;

+            }

+

+            if (lsp_is_external(op->nbsp) || op->has_unknown) {

+                return;

+            }

+

+            for (size_t i = 0; i < op->n_lsp_addrs; i++) {

+                for (size_t j = 0; j < op->lsp_addrs[i].n_ipv4_addrs; j++) {

+                    ds_clear(match);

+                    ds_put_format(match, "arp.tpa == %s && arp.op == 1",

+                                op->lsp_addrs[i].ipv4_addrs[j].addr_s);

+                    ds_clear(actions);

+                    ds_put_format(actions,

+                        "eth.dst = eth.src; "

+                        "eth.src = %s; "

+                        "arp.op = 2; /* ARP reply */ "

+                        "arp.tha = arp.sha; "

+                        "arp.sha = %s; "

+                        "arp.tpa = arp.spa; "

+                        "arp.spa = %s; "

+                        "outport = inport; "

+                        "flags.loopback = 1; "

+                        "output;",

+                        op->lsp_addrs[i].ea_s, op->lsp_addrs[i].ea_s,

+                        op->lsp_addrs[i].ipv4_addrs[j].addr_s);

+                    ovn_lflow_add_with_hint(lflows, op->od,

+                                            S_SWITCH_IN_ARP_ND_RSP, 50,

+                                            ds_cstr(match),

+                                            ds_cstr(actions),

+                                            &op->nbsp->header_);

+

+                    /* Do not reply to an ARP request from the port that owns

+                     * the address (otherwise a DHCP client that ARPs to check

+                     * for a duplicate address will fail).  Instead, forward

+                     * it the usual way.

+                     *

+                     * (Another alternative would be to simply drop the packet.

+                     * If everything is working as it is configured, then this

+                     * would produce equivalent results, since no one should

+                     * reply to the request.  But ARPing for one's own IP

+                     * address is intended to detect situations where the

+                     * network is not working as configured, so dropping the

+                     * request would frustrate that intent.) */

+                    ds_put_format(match, " && inport == %s", op->json_key);

+                    ovn_lflow_add_with_hint(lflows, op->od,

+                                            S_SWITCH_IN_ARP_ND_RSP, 100,

+                                            ds_cstr(match), "next;",

+                                            &op->nbsp->header_);

+                }

+

+                /* For ND solicitations, we need to listen for both the

+                 * unicast IPv6 address and its all-nodes multicast address,

+                 * but always respond with the unicast IPv6 address. */

+                for (size_t j = 0; j < op->lsp_addrs[i].n_ipv6_addrs; j++) {

+                    ds_clear(match);

+                    ds_put_format(match,

+                            "nd_ns && ip6.dst == {%s, %s} && nd.target == %s",

+                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

+                            op->lsp_addrs[i].ipv6_addrs[j].sn_addr_s,

+                            op->lsp_addrs[i].ipv6_addrs[j].addr_s);

+

+                    ds_clear(actions);

+                    ds_put_format(actions,

+                            "%s { "

+                            "eth.src = %s; "

+                            "ip6.src = %s; "

+                            "nd.target = %s; "

+                            "nd.tll = %s; "

+                            "outport = inport; "

+                            "flags.loopback = 1; "

+                            "output; "

+                            "};",

+                            lsp_is_router(op->nbsp) ? "nd_na_router" : "nd_na",

+                            op->lsp_addrs[i].ea_s,

+                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

+                            op->lsp_addrs[i].ipv6_addrs[j].addr_s,

+                            op->lsp_addrs[i].ea_s);

+                    ovn_lflow_add_with_hint(lflows, op->od,

+                                            S_SWITCH_IN_ARP_ND_RSP, 50,

+                                            ds_cstr(match),

+                                            ds_cstr(actions),

+                                            &op->nbsp->header_);

+

+                    /* Do not reply to a solicitation from the port that owns

+                     * the address (otherwise DAD detection will fail). */

+                    ds_put_format(match, " && inport == %s", op->json_key);

+                    ovn_lflow_add_with_hint(lflows, op->od,

+                                            S_SWITCH_IN_ARP_ND_RSP, 100,

+                                            ds_cstr(match), "next;",

+                                            &op->nbsp->header_);

+                }

+            }

+        }

+    }

+}

+

+/* Ingress table 13: ARP/ND responder, by default goto next.

+ * (priority 0)*/

+static void

+build_lswitch_arp_nd_responder_default(struct ovn_datapath *od,

+                                       struct hmap *lflows)

+{

+    if (od->nbs) {

+        ovn_lflow_add(lflows, od, S_SWITCH_IN_ARP_ND_RSP, 0, "1", "next;");

+    }

+}

+

+/* Ingress table 13: ARP/ND responder for service monitor source ip.

+ * (priority 110)*/

+static void

+build_lswitch_arp_nd_service_monitor(struct ovn_northd_lb *lb,

+                                     struct hmap *lflows,

+                                     struct ds *actions,

+                                     struct ds *match)

+{

+    for (size_t i = 0; i < lb->n_vips; i++) {

+        struct ovn_northd_lb_vip *lb_vip_nb = &lb->vips_nb[i];

+        if (!lb_vip_nb->lb_health_check) {

+            continue;

+        }

+

+        for (size_t j = 0; j < lb_vip_nb->n_backends; j++) {

+            struct ovn_northd_lb_backend *backend_nb =

+                &lb_vip_nb->backends_nb[j];

+            if (!backend_nb->op || !backend_nb->svc_mon_src_ip) {

+                continue;

+            }

+

+            ds_clear(match);

+            ds_put_format(match, "arp.tpa == %s && arp.op == 1",

+                          backend_nb->svc_mon_src_ip);

+            ds_clear(actions);

+            ds_put_format(actions,

+                "eth.dst = eth.src; "

+                "eth.src = %s; "

+                "arp.op = 2; /* ARP reply */ "

+                "arp.tha = arp.sha; "

+                "arp.sha = %s; "

+                "arp.tpa = arp.spa; "

+                "arp.spa = %s; "

+                "outport = inport; "

+                "flags.loopback = 1; "

+                "output;",

+                svc_monitor_mac, svc_monitor_mac,

+                backend_nb->svc_mon_src_ip);

+            ovn_lflow_add_with_hint(lflows,

+                                    backend_nb->op->od,

+                                    S_SWITCH_IN_ARP_ND_RSP, 110,

+                                    ds_cstr(match), ds_cstr(actions),

+                                    &lb->nlb->header_);

+        }

+    }

+}

+

 /* Returns a string of the IP address of the router port 'op' that

  * overlaps with 'ip_s".  If one is not found, returns NULL.

@@ -11322,6 +11334,7 @@ build_lswitch_and_lrouter_iterate_by_od(struct ovn_datapath *od,

     build_fwd_group_lflows(od, lsi->lflows);

     build_lswitch_lflows_admission_control(od, lsi->lflows);

     build_lswitch_input_port_sec_od(od, lsi->lflows);

+    build_lswitch_arp_nd_responder_default(od, lsi->lflows);

     /* Build Logical Router Flows. */

     build_adm_ctrl_flows_for_lrouter(od, lsi->lflows);

@@ -11352,7 +11365,12 @@ build_lswitch_and_lrouter_iterate_by_op(struct ovn_port *op,

     /* Build Logical Switch Flows. */

     build_lswitch_input_port_sec_op(op, lsi->lflows, &lsi->actions,

                                     &lsi->match);

-

+    build_lswitch_arp_nd_responder_skip_local(op, lsi->lflows,

+                                              &lsi->match);

+    build_lswitch_arp_nd_responder_known_ips(op, lsi->lflows,

+                                             lsi->ports,

+                                             &lsi->actions,

+                                             &lsi->match);

     /* Build Logical Router Flows. */

     build_adm_ctrl_flows_for_lrouter_port(op, lsi->lflows, &lsi->match,

                                           &lsi->actions);

@@ -11379,6 +11397,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,

 {

     struct ovn_datapath *od;

     struct ovn_port *op;

+    struct ovn_northd_lb *lb;

     char *svc_check_match = xasprintf("eth.dst == %s", svc_monitor_mac);

@@ -11405,6 +11424,11 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,

     HMAP_FOR_EACH (op, key_node, ports) {

         build_lswitch_and_lrouter_iterate_by_op(op, &lsi);

     }

+    HMAP_FOR_EACH (lb, hmap_node, lbs) {

+        build_lswitch_arp_nd_service_monitor(lb, lsi.lflows,

+                                             &lsi.actions,

+                                             &lsi.match);

+    }

     free(svc_check_match);

     ds_destroy(&lsi.match);

@@ -11412,7 +11436,7 @@ build_lswitch_and_lrouter_flows(struct hmap *datapaths, struct hmap *ports,

     /* Legacy lswitch build - to be migrated. */

     build_lswitch_flows(datapaths, ports, lflows, mcgroups,

-                        igmp_groups, lbs);

+                        igmp_groups);

     /* Legacy lrouter build - to be migrated. */

     build_lrouter_flows(datapaths, ports, lflows, meter_groups, lbs);

-- 

2.29.2