ovn: Set proper Neighbour Adv flag when replying for NS request for router IP

Presently when a VM's IPv6 stack sends a Neighbor Solicitation request for its
router IP, (mostly when the ND cache entry for the router is in STALE state)
ovn-controller responds with a Neighbor Adv packet (using the action nd_na).
But it doesn't set 'ND_RSO_ROUTER' in the RSO flags (please see RFC4861 page 23).
Because of which, the VM deletes the default route. The default route gets added
again when the next RA is received (but would again gets deleted if its sends
NS request). And this results in disruption of IPv6 traffic.

This patch addresses this issue by adding a new action 'nd_na_router' which is
same as 'nd_na' but it sets the 'ND_RSO_ROUTER' in the RSO flags. ovn-northd
uses this action. A new action is added instead of modifying the existing 'nd_na'
action. This is because
  - We cannot set the RSO flags in the "nd_na { ..actions .. }"
  - It would be ugly to have something like nd_na { router_flags, ...actions .. }

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1567735
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
Acked-by: Mark Michelson <mmichels@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

1 files changed, 15 insertions, 8 deletions

diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
index 6e6aa1caa..305f20649 100644
--- a/ovn/controller/pinctrl.c
+++ b/ovn/controller/pinctrl.c
@@ -81,7 +81,8 @@ static void send_garp_run(struct controller_ctx *ctx,
                           struct sset *active_tunnels);
 static void pinctrl_handle_nd_na(const struct flow *ip_flow,
                                  const struct match *md,
-                                 struct ofpbuf *userdata);
+                                 struct ofpbuf *userdata,
+                                 bool is_router);
 static void reload_metadata(struct ofpbuf *ofpacts,
                             const struct match *md);
 static void pinctrl_handle_put_nd_ra_opts(
@@ -1154,7 +1155,11 @@ process_packet_in(const struct ofp_header *msg, struct controller_ctx *ctx)
         break;
 
     case ACTION_OPCODE_ND_NA:
-        pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata);
+        pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata, false);
+        break;
+
+    case ACTION_OPCODE_ND_NA_ROUTER:
+        pinctrl_handle_nd_na(&headers, &pin.flow_metadata, &userdata, true);
         break;
 
     case ACTION_OPCODE_PUT_ND:
@@ -2308,7 +2313,7 @@ reload_metadata(struct ofpbuf *ofpacts, const struct match *md)
 
 static void
 pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
-                     struct ofpbuf *userdata)
+                     struct ofpbuf *userdata, bool is_router)
 {
     /* This action only works for IPv6 ND packets, and the switch should only
      * send us ND packets this way, but check here just to be sure. */
@@ -2322,13 +2327,15 @@ pinctrl_handle_nd_na(const struct flow *ip_flow, const struct match *md,
     struct dp_packet packet;
     dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
 
-    /* xxx These flags are not exactly correct.  Look at section 7.2.4
-     * xxx of RFC 4861.  For example, we need to set ND_RSO_ROUTER for
-     * xxx router's interfaces and ND_RSO_SOLICITED only if it was
-     * xxx requested. */
+    /* These flags are not exactly correct.  Look at section 7.2.4
+     * of RFC 4861. */
+    uint32_t rso_flags = ND_RSO_SOLICITED | ND_RSO_OVERRIDE;
+    if (is_router) {
+        rso_flags |= ND_RSO_ROUTER;
+    }
     compose_nd_na(&packet, ip_flow->dl_dst, ip_flow->dl_src,
                   &ip_flow->nd_target, &ip_flow->ipv6_src,
-                  htonl(ND_RSO_SOLICITED | ND_RSO_OVERRIDE));
+                  htonl(rso_flags));
 
     /* Reload previous packet metadata and set actions from userdata. */
     set_actions_and_enqueue_msg(&packet, md, userdata);