[ovs-dev] [PATCH v2] ovn: Fix the issue in IPv6 Neigh Solicitation responder for router IPs

[nusiddiq at redhat.com]

From: Numan Siddique <nusiddiq at redhat.com>

Commit [1] added a new action 'nd_na_router' to set the router bit
in the 'flags' field of the Neighbour Adv packet for router IPs.
This action was used in the router pipeline. But the logical switch
pipeline also adds the Neighbour Adv flows for router IPs but with
'nd_na' action (which the commit [1] didn't handle).

This patch fixes this by changing the action to 'nd_na_router' for
router IPs.

Without this patch, the IPv6 functionality is broken.

[1] - "c9756229ed: ovn: Set proper Neighbour Adv flag when replying
for NS request for router IP"

Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
---
 ovn/northd/ovn-northd.8.xml | 24 ++++++++++++++++++++++--
 ovn/northd/ovn-northd.c     |  4 +++-
 tests/ovn.at                | 15 ++++++++++++++-
 3 files changed, 39 insertions(+), 4 deletions(-)

v1 -> v2
-------
Addressed the review comments from Han Zhou by using the nd_na_router
action for router IPs in the logical switch pipeline.


diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index e5ff2b661..7352c6764 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -549,8 +549,8 @@ output;
         <p>
           Priority-50 flows that match IPv6 ND neighbor solicitations to
           each known IP address <var>A</var> (and <var>A</var>'s
-          solicited node address) of every logical switch port, and
-          respond with neighbor advertisements directly with
+          solicited node address) of every logical switch port except of type
+          router, and respond with neighbor advertisements directly with
           corresponding Ethernet address <var>E</var>:
         </p>
 
@@ -566,6 +566,26 @@ nd_na {
 };
         </pre>
 
+        <p>
+          Priority-50 flows that match IPv6 ND neighbor solicitations to
+          each known IP address <var>A</var> (and <var>A</var>'s
+          solicited node address) of logical switch port of type router, and
+          respond with neighbor advertisements directly with
+          corresponding Ethernet address <var>E</var>:
+        </p>
+
+        <pre>
+nd_na_router {
+    eth.src = <var>E</var>;
+    ip6.src = <var>A</var>;
+    nd.target = <var>A</var>;
+    nd.tll = <var>E</var>;
+    outport = inport;
+    flags.loopback = 1;
+    output;
+};
+        </pre>
+
         <p>
           These flows are omitted for logical ports (other than router ports or
           <code>localport</code> ports) that are down.
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 1d020a739..72e25181d 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -4163,7 +4163,7 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
 
                 ds_clear(&actions);
                 ds_put_format(&actions,
-                        "nd_na { "
+                        "%s { "
                         "eth.src = %s; "
                         "ip6.src = %s; "
                         "nd.target = %s; "
@@ -4172,6 +4172,8 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap *ports,
                         "flags.loopback = 1; "
                         "output; "
                         "};",
+                        !strcmp(op->nbsp->type, "router") ?
+                            "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,
diff --git a/tests/ovn.at b/tests/ovn.at
index c5d054c21..e10a7f9ba 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9530,7 +9530,7 @@ ovn-nbctl lr-add lr0_ip6
 ovn-nbctl lrp-add lr0_ip6 lrp0_ip6 00:00:00:00:af:01 aef0:0:0:0:0:0:0:0/64
 ovn-nbctl lsp-add sw0_ip6 lrp0_ip6-attachment
 ovn-nbctl lsp-set-type lrp0_ip6-attachment router
-ovn-nbctl lsp-set-addresses lrp0_ip6-attachment 00:00:00:00:af:01
+ovn-nbctl lsp-set-addresses lrp0_ip6-attachment router
 ovn-nbctl lsp-set-options lrp0_ip6-attachment router-port=lrp0_ip6
 ovn-nbctl set logical_router_port lrp0_ip6 ipv6_ra_configs:address_mode=slaac
 
@@ -9563,6 +9563,19 @@ ovs-vsctl -- add-port br-int hv1-vif1 -- \
 ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
 
 OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up sw0_ip6-port1` = xup])
+
+# There should be 2 Neighbor Advertisement flows for the router port
+# aef0:: ip address in logical switch pipeline with action nd_na_router.
+AT_CHECK([ovn-sbctl dump-flows sw0_ip6 | grep ls_in_arp_rsp | \
+grep "nd_na_router" | wc -l], [0], [2
+])
+
+# There should be 4 Neighbor Advertisement flows with action nd_na_router
+# in the router pipeline for the router lr0_ip6.
+AT_CHECK([ovn-sbctl dump-flows lr0_ip6 | grep nd_na_router | \
+wc -l], [0], [4
+])
+
 cr_uuid=`ovn-sbctl find port_binding logical_port=cr-ip6_public | grep _uuid | cut -f2 -d ":"`
 
 # There is only one chassis.
-- 
2.17.1