diff mbox series

[ovs-dev,branch,2.9] ovn: Fix the issue in IPv6 Neigh Solicitation responder for router IPs

Message ID 20180827203107.957-1-nusiddiq@redhat.com
State Accepted
Headers show
Series [ovs-dev,branch,2.9] ovn: Fix the issue in IPv6 Neigh Solicitation responder for router IPs | expand

Commit Message

Numan Siddique Aug. 27, 2018, 8:31 p.m. UTC
From: Numan Siddique <nusiddiq@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@redhat.com>
Acked-by: Han Zhou <hzhou8@ebay.com>

(cherry picked from commit bec7c6415d1d770c90ff32e3626c0f63d55763af)
Conflicts:
	tests/ovn.at
---
 ovn/northd/ovn-northd.8.xml | 24 ++++++++++++++++++++++--
 ovn/northd/ovn-northd.c     |  4 +++-
 tests/ovn.at                | 22 +++++++++++++++++++++-
 3 files changed, 46 insertions(+), 4 deletions(-)

Comments

Ben Pfaff Aug. 27, 2018, 8:38 p.m. UTC | #1
On Tue, Aug 28, 2018 at 02:01:07AM +0530, nusiddiq@redhat.com wrote:
> From: Numan Siddique <nusiddiq@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@redhat.com>
> Acked-by: Han Zhou <hzhou8@ebay.com>

Thanks for the backport.  Applied to branch-2.9.
diff mbox series

Patch

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 7cd7a10ed..78df522c9 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -544,8 +544,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>
 
@@ -561,6 +561,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 56bddffcd..9ae4daafe 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -3733,7 +3733,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; "
@@ -3742,6 +3742,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 460f5f5e2..add506a54 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -9192,7 +9192,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
 
@@ -9228,6 +9228,26 @@  ovs-vsctl set open . external-ids:ovn-bridge-mappings=phys:br-phys
 # XXX This should be more systematic.
 sleep 1
 
+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.
+chassis_uuid=`ovn-sbctl list chassis | grep _uuid | cut -f2 -d ":"`
+OVS_WAIT_UNTIL([test $chassis_uuid = `ovn-sbctl get port_binding $cr_uuid chassis`])
+
 trim_zeros() {
     sed 's/\(00\)\{1,\}$//'
 }