UCT: prioritize routes with longer subnet masks in the reachability check

src/ucs/sys/netlink.c

@@ -28,9 +28,7 @@
 typedef struct {
     const struct sockaddr *sa_remote;
     int                    if_index;
-    int                    found;
-    int                    allow_default_gw; /* Allow matching default
-                                                gateway routes */
+    int                    netmask_len;
 } ucs_netlink_route_info_t;
 
 
@@ -249,44 +247,29 @@ ucs_netlink_parse_rt_entry_cb(const struct nlmsghdr *nlh, void *arg)
     return UCS_INPROGRESS;
 }
 
-static void ucs_netlink_lookup_route(ucs_netlink_route_info_t *info)
+static int
+ucs_netlink_lookup_in_iface_rules(const struct sockaddr *sa_remote,
+                                  ucs_netlink_rt_rules_t *iface_rules)
 {
-    ucs_netlink_rt_rules_t *iface_rules;
+    int found_netmask_len = -1;
     ucs_netlink_route_entry_t *curr_entry;
-    khiter_t iter;
-
-    iter = kh_get(ucs_netlink_rt_cache, &ucs_netlink_routing_table_cache,
-                  info->if_index);
-    if (iter == kh_end(&ucs_netlink_routing_table_cache)) {
-        info->found = 0;
-        return;
-    }
 
-    iface_rules = &kh_val(&ucs_netlink_routing_table_cache, iter);
     ucs_array_for_each(curr_entry, iface_rules) {
-
-        if ((curr_entry->subnet_prefix_len == 0) && !info->allow_default_gw) {
-            ucs_trace("iface_index=%d: skipping default gateway route",
-                      info->if_index);
-            continue;
-        }
-
-        if (ucs_sockaddr_is_same_subnet(
-                                info->sa_remote,
-                                (const struct sockaddr *)&curr_entry->dest,
-                                curr_entry->subnet_prefix_len)) {
-            info->found = 1;
-            return;
+        if ((curr_entry->subnet_prefix_len > found_netmask_len) &&
+            ucs_sockaddr_is_same_subnet(
+                    sa_remote, (const struct sockaddr*)&curr_entry->dest,
+                    curr_entry->subnet_prefix_len)) {
+            found_netmask_len = curr_entry->subnet_prefix_len;
         }
     }
+
+    return found_netmask_len;
 }
 
-int ucs_netlink_route_exists(int if_index, const struct sockaddr *sa_remote,
-                             int allow_default_gw)
+static void ucs_netlink_init_routing_table_cache(void)
 {
     static ucs_init_once_t init_once = UCS_INIT_ONCE_INITIALIZER;
     struct rtmsg rtm                 = {0};
-    ucs_netlink_route_info_t info;
 
     UCS_INIT_ONCE(&init_once) {
         rtm.rtm_table  = RT_TABLE_UNSPEC; /* fetch all the tables */
@@ -300,13 +283,67 @@ int ucs_netlink_route_exists(int if_index, const struct sockaddr *sa_remote,
                                  sizeof(rtm), ucs_netlink_parse_rt_entry_cb,
                                  NULL);
     }
+}
 
-    info.if_index         = if_index;
-    info.sa_remote        = sa_remote;
-    info.found            = 0;
-    info.allow_default_gw = allow_default_gw;
+static void ucs_netlink_lookup_route(ucs_netlink_route_info_t *info)
+{
+    ucs_netlink_rt_rules_t *iface_rules;
+    khiter_t iter;
+
+    ucs_netlink_init_routing_table_cache();
+
+    iter = kh_get(ucs_netlink_rt_cache, &ucs_netlink_routing_table_cache,
+                  info->if_index);
+    if (iter == kh_end(&ucs_netlink_routing_table_cache)) {
+        return;
+    }
+
+    iface_rules       = &kh_val(&ucs_netlink_routing_table_cache, iter);
+    info->netmask_len = ucs_netlink_lookup_in_iface_rules(info->sa_remote,
+                                                          iface_rules);
+}
+
+static int ucs_netlink_max_netmask_len(const struct sockaddr *sa_remote)
+{
+    int max_netmask_len = -1;
+    ucs_netlink_rt_rules_t iface_rules;
+
+    kh_foreach_value(&ucs_netlink_routing_table_cache, iface_rules, {
+        int curr_netmask_len = ucs_netlink_lookup_in_iface_rules(sa_remote,
+                                                                 &iface_rules);
+        if (curr_netmask_len > max_netmask_len) {
+            max_netmask_len = curr_netmask_len;
+        }
+    })
+
+    return max_netmask_len;
+}
+
+int ucs_netlink_route_exists(int if_index, const struct sockaddr *sa_remote,
+                             int *netmask_len_p)
+{
+    ucs_netlink_route_info_t info = {
+        .if_index    = if_index,
+        .sa_remote   = sa_remote,
+        .netmask_len = -1
+    };
 
     ucs_netlink_lookup_route(&info);
 
-    return info.found;
+    if (netmask_len_p != NULL) {
+        *netmask_len_p = info.netmask_len;
+    }
+
+    return (info.netmask_len > -1);
+}
+
+int ucs_netlink_is_best_route(int if_index, const struct sockaddr *sa_remote)
+{
+    int netmask_len;
+
+    if (!ucs_netlink_route_exists(if_index, sa_remote, &netmask_len)) {
+        return 0;
+    }
+
+    return (ucs_netlink_max_netmask_len(sa_remote) == netmask_len);
 }

src/ucs/sys/netlink.h

@@ -45,20 +45,33 @@ ucs_netlink_send_request(int protocol, unsigned short nlmsg_type,
 
 
 /**
- * Check whether a routing table rule exists for a given network
- * interface name and a destination address.
+ * Check whether a route exists for a given network interface and
+ * destination address.
  *
- * @param [in]  if_index          A global index representing the network
-                                  interface, as assigned by the system
-                                  (e.g., obtained via if_nametoindex()).
- * @param [in]  sa_remote         Pointer to the destination address.
- * @param [in]  allow_default_gw  Allow matching default gateway routes (1) or
- *                                only specific subnet routes (0).
+ * @param [in]  if_index           A global index representing the network
+ *                                 interface, as assigned by the system
+ *                                 (e.g., obtained via if_nametoindex()).
+ * @param [in]  sa_remote          Pointer to the destination address.
+ * @param [out] netmask_len_p      Optional pointer to store the netmask length
+ *                                 of the route found. Pass NULL if this
+ *                                 information is not needed.
  *
- * @return 1 if rule exists, or 0 otherwise.
+ * @return 1 if a route exists, or 0 otherwise.
  */
 int ucs_netlink_route_exists(int if_index, const struct sockaddr *sa_remote,
-                             int allow_default_gw);
+                             int *netmask_len_p);
+
+/**
+ * Check if this network interface has the best route to the destination
+ * address.
+ *
+ * @param [in]  if_index         Network interface index.
+ * @param [in]  sa_remote        Pointer to the destination address.
+ *
+ * @return 1 if this network interface has the best route to the destination
+ *         address, or 0 otherwise.
+ */
+int ucs_netlink_is_best_route(int if_index, const struct sockaddr *sa_remote);
 
 END_C_DECLS
 

src/ucs/sys/sys.c

@@ -178,21 +178,6 @@ ucs_status_t ucs_ifname_to_index(const char *ndev_name, unsigned *ndev_index_p)
     return UCS_OK;
 }
 
-int ucs_netif_is_ipoib(const char *if_name)
-{
-    struct ifreq ifr;
-    ucs_status_t status;
-
-    status = ucs_netif_ioctl(if_name, SIOCGIFHWADDR, &ifr);
-    if (status != UCS_OK) {
-        /* If we can't determine the hardware type, assume it's not IPoIB */
-        ucs_debug("failed to get hardware address for %s", if_name);
-        return 0;
-    }
-
-    return ifr.ifr_hwaddr.sa_family == ARPHRD_INFINIBAND;
-}
-
 static uint64_t ucs_get_mac_address()
 {
     static uint64_t mac_address = 0;

src/ucs/sys/sys.h

@@ -192,16 +192,6 @@ uint32_t ucs_file_checksum(const char *filename);
 ucs_status_t ucs_ifname_to_index(const char *ndev_name, unsigned *ndev_index_p);
 
 
-/**
- * Check if a network interface is an IPoIB (IP over InfiniBand) device.
- *
- * @param [in]  if_name  Network interface name to check.
- *
- * @return 1 if the interface is IPoIB, 0 otherwise.
- */
-int ucs_netif_is_ipoib(const char *if_name);
-
-
 /**
  * Get a globally unique identifier of the machine running the current process.
  */

src/uct/ib/base/ib_iface.c

@@ -710,32 +710,33 @@ uct_ib_iface_roce_is_routable(uct_ib_iface_t *iface, uint8_t gid_index,
         return 0;
     }
 
-    if (!ucs_netlink_route_exists(ndev_index, sa_remote, 1)) {
-        /* try to use loopback interface for reachability check, because it may
-         * be used for routing in case of an interface with VRF is configured
-         * and a RoCE IP interface uses this VRF table for routing.
-         */
-        if (uct_ib_iface_get_loopback_ndev_index(&lo_ndev_index) != UCS_OK) {
-            uct_iface_fill_info_str_buf(params,
-                                        "loopback iface index is not found");
-            return 0;
-        }
+    if (ucs_netlink_is_best_route(ndev_index, sa_remote)) {
+        /* This interface has the best route */
+        return 1;
+    }
 
-        if (!ucs_netlink_route_exists(lo_ndev_index, sa_remote, 1)) {
-            uct_iface_fill_info_str_buf(params,
-                                        "remote address %s is not routable "
-                                        "neither by interface "UCT_IB_IFACE_FMT
-                                        " (ifname_index=%u) nor by loopback "
-                                        "interface (ifname_index=%u)",
-                                        ucs_sockaddr_str(sa_remote, remote_str,
-                                                         128),
-                                        UCT_IB_IFACE_ARG(iface),
-                                        ndev_index, lo_ndev_index);
-            return 0;
-        }
+    /* No route found - try loopback interface for reachability check,
+     * because it may be used for routing in case of an interface with
+     * VRF is configured and a RoCE IP interface uses this VRF table for
+     * routing. */
+    if ((uct_ib_iface_get_loopback_ndev_index(&lo_ndev_index) == UCS_OK) &&
+        ucs_netlink_route_exists(lo_ndev_index, sa_remote, NULL)) {
+        ucs_trace(UCT_IB_IFACE_FMT ": found specific route via loopback to %s",
+                  UCT_IB_IFACE_ARG(iface),
+                  ucs_sockaddr_str(sa_remote, remote_str, sizeof(remote_str)));
+        return 1;
     }
 
-    return 1;
+    uct_iface_fill_info_str_buf(params,
+                                "remote address %s is not routable "
+                                "neither by interface " UCT_IB_IFACE_FMT
+                                " (ifname_index=%u) nor by loopback "
+                                "interface (ifname_index=%u)",
+                                ucs_sockaddr_str(sa_remote, remote_str,
+                                                 sizeof(remote_str)),
+                                UCT_IB_IFACE_ARG(iface), ndev_index,
+                                lo_ndev_index);
+    return 0;
 }
 
 static int

src/uct/tcp/tcp.h

@@ -295,12 +295,7 @@ typedef enum uct_tcp_device_addr_flags {
      * Device address is extended by additional information:
      * @ref uct_iface_local_addr_ns_t for loopback reachability
      */
-    UCT_TCP_DEVICE_ADDR_FLAG_LOOPBACK         = UCS_BIT(0),
-
-    /**
-     * Allow communication with default gateway
-     */
-    UCT_TCP_DEVICE_ADDR_FLAG_ALLOW_DEFAULT_GW = UCS_BIT(1)
+    UCT_TCP_DEVICE_ADDR_FLAG_LOOPBACK = UCS_BIT(0)
 } uct_tcp_device_addr_flags_t;
 
 

src/uct/tcp/tcp_iface.c

@@ -139,11 +139,6 @@ static ucs_status_t uct_tcp_iface_get_device_address(uct_iface_h tl_iface,
     dev_addr->flags     = 0;
     dev_addr->sa_family = saddr->sa_family;
 
-    /* Default gateway is not relevant for IPoIB interfaces */
-    if (!ucs_netif_is_ipoib(iface->if_name)) {
-        dev_addr->flags |= UCT_TCP_DEVICE_ADDR_FLAG_ALLOW_DEFAULT_GW;
-    }
-
     if (ucs_sockaddr_is_inaddr_loopback(saddr)) {
         dev_addr->flags |= UCT_TCP_DEVICE_ADDR_FLAG_LOOPBACK;
         memset(pack_ptr, 0, sizeof(uct_iface_local_addr_ns_t));
@@ -211,7 +206,6 @@ uct_tcp_iface_is_reachable_v2(const uct_iface_h tl_iface,
     struct sockaddr_storage remote_addr;
     char remote_addr_str[UCS_SOCKADDR_STRING_LEN];
     unsigned ndev_index;
-    int allow_default_gw;
     ucs_status_t status;
 
     if (!uct_iface_is_reachable_params_valid(
@@ -270,19 +264,20 @@ uct_tcp_iface_is_reachable_v2(const uct_iface_h tl_iface,
         return 0;
     }
 
-    allow_default_gw = !!(tcp_dev_addr->flags &
-                          UCT_TCP_DEVICE_ADDR_FLAG_ALLOW_DEFAULT_GW);
-
-    if (!ucs_netlink_route_exists(ndev_index,
-                                  (const struct sockaddr *)&remote_addr,
-                                  allow_default_gw)) {
+    if (!ucs_netlink_is_best_route(ndev_index,
+                                   (const struct sockaddr*)&remote_addr)) {
         uct_iface_fill_info_str_buf(
                     params, "no route to %s",
                     ucs_sockaddr_str((const struct sockaddr *)&remote_addr,
                                      remote_addr_str, UCS_SOCKADDR_STRING_LEN));
         return 0;
     }
 
+    /* This interface has the best route */
+    ucs_trace("tcp_iface %p (%s): this interface has the best route to %s",
+              iface, iface->if_name,
+              ucs_sockaddr_str((const struct sockaddr*)&remote_addr,
+                               remote_addr_str, UCS_SOCKADDR_STRING_LEN));
     return 1;
 }