From: Numan Siddique <nusid...@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 in the router
pipeline. But the logical switch pipeline was also adding the
Neighbour Adv flows for router IPs with 'nd_na' action, which the
commit [1] didn't handle.

This patch fixes this.

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

Signed-off-by: Numan Siddique <nusid...@redhat.com>
---
 ovn/northd/ovn-northd.c | 70 ++++++++++++++++++++++-------------------
 tests/ovn.at            | 24 +++++++++++++-
 2 files changed, 61 insertions(+), 33 deletions(-)

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 1d020a739..acc0d586e 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -4150,40 +4150,46 @@ build_lswitch_flows(struct hmap *datapaths, struct hmap 
*ports,
                               ds_cstr(&match), "next;");
             }
 
-            /* 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);
+            /* Skip adding Neighbor Adv flows for ports with router peers.
+             * Router pipeline takes care of adding those with nd_na_router
+             * action.
+             */
+            if (!op->peer) {
+                /* 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,
-                        "nd_na { "
-                        "eth.src = %s; "
-                        "ip6.src = %s; "
-                        "nd.target = %s; "
-                        "nd.tll = %s; "
-                        "outport = inport; "
-                        "flags.loopback = 1; "
-                        "output; "
-                        "};",
-                        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(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
-                              ds_cstr(&match), ds_cstr(&actions));
+                    ds_clear(&actions);
+                    ds_put_format(&actions,
+                            "nd_na { "
+                            "eth.src = %s; "
+                            "ip6.src = %s; "
+                            "nd.target = %s; "
+                            "nd.tll = %s; "
+                            "outport = inport; "
+                            "flags.loopback = 1; "
+                            "output; "
+                            "};",
+                            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(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 50,
+                                  ds_cstr(&match), ds_cstr(&actions));
 
-                /* 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(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 100,
-                              ds_cstr(&match), "next;");
+                    /* 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(lflows, op->od, S_SWITCH_IN_ARP_ND_RSP, 100,
+                                  ds_cstr(&match), "next;");
+                }
             }
         }
     }
diff --git a/tests/ovn.at b/tests/ovn.at
index c5d054c21..2894ce28c 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,28 @@ 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 no Neighbor Advertisement flow for the router port
+# aef0:: ip address in logical switch pipeline.
+AT_CHECK([ovn-sbctl dump-flows sw0_ip6 | grep ls_in_arp_rsp | \
+grep "priority=50" | grep "nd.target == aef0::" | wc -l], [0], [1
+])
+
+# There should no Neighbor Advertisement flow for link local
+# address (fe80::200:ff:fe00:af01) of the router port in logical switch
+# pipeline.
+
+AT_CHECK([ovn-sbctl dump-flows sw0_ip6 | grep ls_in_arp_rsp | \
+grep "priority=50" | grep "nd.target == fe80::200:ff:fe00:af01" | wc -l],
+[0], [0
+])
+
+# 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

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to