From f6be07c457385cfacd9b802e4cade9f6f6ab7d6f Mon Sep 17 00:00:00 2001 From: Serguei Smirnov Date: Fri, 26 May 2023 10:42:23 -0700 Subject: [PATCH] LU-16850 socklnd: remove ksnr_myiface from ksock_conn_cb Drop ksnr_myiface: it is no longer needed since socklnd TCP bonding got removed. There's one interface per connection cb per peer_ni, and it can be accessed as net->ksnn_interface.ksni_index. Fix setting of ksni_nroutes accordingly. Duplication of interface index in conn_cb and ksnn_interface was causing the assertion ASSERTION( net->ksnn_interface.ksni_nroutes == 0 ) in ksocknal_shutdown() to fail if the corresponding device is deregistered before lnd shutdown. Modify test_214 of sanity-lnet to create connections so that the scenario of socklnd shutdown with NI on a deregistered interface is covered. Fixes: 3c9282a6 ("LU-16378 lnet: handles unregister/register events) Test-Parameters: trivial Signed-off-by: Serguei Smirnov Change-Id: I4de164c9e64aa770164a8320b9460fadce49aa06 Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/51148 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Frank Sehr Reviewed-by: Cyril Bordage Reviewed-by: Oleg Drokin --- lnet/klnds/socklnd/socklnd.c | 131 +++------------------------------------- lnet/klnds/socklnd/socklnd.h | 1 - lnet/klnds/socklnd/socklnd_cb.c | 4 +- lustre/tests/sanity-lnet.sh | 3 + 4 files changed, 15 insertions(+), 124 deletions(-) diff --git a/lnet/klnds/socklnd/socklnd.c b/lnet/klnds/socklnd/socklnd.c index 5917654..43b46b7 100644 --- a/lnet/klnds/socklnd/socklnd.c +++ b/lnet/klnds/socklnd/socklnd.c @@ -46,80 +46,6 @@ static const struct lnet_lnd the_ksocklnd; struct ksock_nal_data ksocknal_data; -static struct ksock_interface * -ksocknal_index2iface(struct lnet_ni *ni, int index) -{ - struct ksock_net *net = ni->ni_data; - struct ksock_interface *iface; - - iface = &net->ksnn_interface; - - if (iface->ksni_index == index) - return iface; - - return NULL; -} - -static int ksocknal_ip2index(struct sockaddr *addr, struct lnet_ni *ni) -{ - struct net_device *dev; - int ret = -1; - DECLARE_CONST_IN_IFADDR(ifa); - - if (addr->sa_family != AF_INET && addr->sa_family != AF_INET6) - return ret; - - rcu_read_lock(); - for_each_netdev(ni->ni_net_ns, dev) { - int flags = dev_get_flags(dev); - struct in_device *in_dev; - - if (flags & IFF_LOOPBACK) /* skip the loopback IF */ - continue; - - if (!(flags & IFF_UP)) - continue; - - switch (addr->sa_family) { - case AF_INET: - in_dev = __in_dev_get_rcu(dev); - if (!in_dev) - continue; - - in_dev_for_each_ifa_rcu(ifa, in_dev) { - if (ifa->ifa_local == - ((struct sockaddr_in *)addr)->sin_addr.s_addr) - ret = dev->ifindex; - } - endfor_ifa(in_dev); - break; -#if IS_ENABLED(CONFIG_IPV6) - case AF_INET6: { - struct inet6_dev *in6_dev; - const struct inet6_ifaddr *ifa6; - struct sockaddr_in6 *addr6 = (struct sockaddr_in6*)addr; - - in6_dev = __in6_dev_get(dev); - if (!in6_dev) - continue; - - list_for_each_entry_rcu(ifa6, &in6_dev->addr_list, if_list) { - if (ipv6_addr_cmp(&ifa6->addr, - &addr6->sin6_addr) == 0) - ret = dev->ifindex; - } - break; - } -#endif /* IS_ENABLED(CONFIG_IPV6) */ - } - if (ret >= 0) - break; - } - rcu_read_unlock(); - - return ret; -} - static struct ksock_conn_cb * ksocknal_create_conn_cb(struct sockaddr *addr) { @@ -135,7 +61,6 @@ ksocknal_create_conn_cb(struct sockaddr *addr) rpc_copy_addr((struct sockaddr *)&conn_cb->ksnr_addr, addr); rpc_set_port((struct sockaddr *)&conn_cb->ksnr_addr, rpc_get_port(addr)); - conn_cb->ksnr_myiface = -1; conn_cb->ksnr_scheduled = 0; conn_cb->ksnr_connecting = 0; conn_cb->ksnr_connected = 0; @@ -333,6 +258,7 @@ ksocknal_get_peer_info(struct lnet_ni *ni, int index, struct ksock_conn_cb *conn_cb; int i; int rc = -ENOENT; + struct ksock_net *net; read_lock(&ksocknal_data.ksnd_global_lock); @@ -358,9 +284,9 @@ ksocknal_get_peer_info(struct lnet_ni *ni, int index, if (conn_cb->ksnr_addr.ss_family == AF_INET) { struct sockaddr_in *sa = (void *)&conn_cb->ksnr_addr; - + net = ni->ni_data; rc = choose_ipv4_src(myip, - conn_cb->ksnr_myiface, + net->ksnn_interface.ksni_index, ntohl(sa->sin_addr.s_addr), ni->ni_net_ns); *peer_ip = ntohl(sa->sin_addr.s_addr); @@ -503,43 +429,10 @@ static void ksocknal_associate_cb_conn_locked(struct ksock_conn_cb *conn_cb, struct ksock_conn *conn) { - struct ksock_peer_ni *peer_ni = conn_cb->ksnr_peer; int type = conn->ksnc_type; - struct ksock_interface *iface; - int conn_iface; - conn_iface = ksocknal_ip2index((struct sockaddr *)&conn->ksnc_myaddr, - peer_ni->ksnp_ni); conn->ksnc_conn_cb = conn_cb; ksocknal_conn_cb_addref(conn_cb); - - if (conn_cb->ksnr_myiface != conn_iface) { - if (conn_cb->ksnr_myiface < 0) { - /* route wasn't bound locally yet (the initial route) */ - CDEBUG(D_NET, "Binding %s %pISc to interface %d\n", - libcfs_idstr(&peer_ni->ksnp_id), - &conn_cb->ksnr_addr, - conn_iface); - } else { - CDEBUG(D_NET, - "Rebinding %s %pISc from interface %d to %d\n", - libcfs_idstr(&peer_ni->ksnp_id), - &conn_cb->ksnr_addr, - conn_cb->ksnr_myiface, - conn_iface); - - iface = ksocknal_index2iface(peer_ni->ksnp_ni, - conn_cb->ksnr_myiface); - if (iface) - iface->ksni_nroutes--; - } - conn_cb->ksnr_myiface = conn_iface; - iface = ksocknal_index2iface(peer_ni->ksnp_ni, - conn_cb->ksnr_myiface); - if (iface) - iface->ksni_nroutes++; - } - ksocknal_incr_conn_count(conn_cb, type); /* Successful connection => further attempts can @@ -564,18 +457,15 @@ ksocknal_add_conn_cb_locked(struct ksock_peer_ni *peer_ni, conn_cb->ksnr_peer = peer_ni; ksocknal_peer_addref(peer_ni); - /* set the conn_cb's interface to the current net's interface */ - conn_cb->ksnr_myiface = net->ksnn_interface.ksni_index; - net->ksnn_interface.ksni_nroutes++; - /* peer_ni's route list takes over my ref on 'route' */ peer_ni->ksnp_conn_cb = conn_cb; + net->ksnn_interface.ksni_nroutes++; list_for_each_entry(conn, &peer_ni->ksnp_conns, ksnc_list) { if (!rpc_cmp_addr((struct sockaddr *)&conn->ksnc_peeraddr, (struct sockaddr *)&conn_cb->ksnr_addr)) continue; - + CDEBUG(D_NET, "call ksocknal_associate_cb_conn_locked\n"); ksocknal_associate_cb_conn_locked(conn_cb, conn); /* keep going (typed conns) */ } @@ -585,9 +475,9 @@ static void ksocknal_del_conn_cb_locked(struct ksock_conn_cb *conn_cb) { struct ksock_peer_ni *peer_ni = conn_cb->ksnr_peer; - struct ksock_interface *iface; struct ksock_conn *conn; struct ksock_conn *cnxt; + struct ksock_net *net; LASSERT(!conn_cb->ksnr_deleted); @@ -599,12 +489,9 @@ ksocknal_del_conn_cb_locked(struct ksock_conn_cb *conn_cb) ksocknal_close_conn_locked(conn, 0); } - if (conn_cb->ksnr_myiface >= 0) { - iface = ksocknal_index2iface(peer_ni->ksnp_ni, - conn_cb->ksnr_myiface); - if (iface) - iface->ksni_nroutes--; - } + net = (struct ksock_net *)(peer_ni->ksnp_ni->ni_data); + net->ksnn_interface.ksni_nroutes--; + LASSERT(net->ksnn_interface.ksni_nroutes >= 0); conn_cb->ksnr_deleted = 1; ksocknal_conn_cb_decref(conn_cb); /* drop peer_ni's ref */ diff --git a/lnet/klnds/socklnd/socklnd.h b/lnet/klnds/socklnd/socklnd.h index 823fd30..b8c1fd9 100644 --- a/lnet/klnds/socklnd/socklnd.h +++ b/lnet/klnds/socklnd/socklnd.h @@ -384,7 +384,6 @@ struct ksock_conn_cb { * can happen next */ time64_t ksnr_retry_interval;/* secs between retries */ - int ksnr_myiface; /* interface index */ struct sockaddr_storage ksnr_addr; /* IP address to connect to */ unsigned int ksnr_scheduled:1;/* scheduled for attention */ unsigned int ksnr_connecting:1;/* connection in progress */ diff --git a/lnet/klnds/socklnd/socklnd_cb.c b/lnet/klnds/socklnd/socklnd_cb.c index 043a655..64b10e8 100644 --- a/lnet/klnds/socklnd/socklnd_cb.c +++ b/lnet/klnds/socklnd/socklnd_cb.c @@ -1917,6 +1917,7 @@ ksocknal_connect(struct ksock_conn_cb *conn_cb) time64_t deadline; bool retry_later = false; int rc = 0; + struct ksock_net *net; deadline = ktime_get_seconds() + ksocknal_timeout(); @@ -1973,8 +1974,9 @@ ksocknal_connect(struct ksock_conn_cb *conn_cb) goto failed; } + net = (struct ksock_net *)(peer_ni->ksnp_ni->ni_data); sock = lnet_connect(&peer_ni->ksnp_id.nid, - conn_cb->ksnr_myiface, + net->ksnn_interface.ksni_index, (struct sockaddr *)&conn_cb->ksnr_addr, peer_ni->ksnp_ni->ni_net_ns); if (IS_ERR(sock)) { diff --git a/lustre/tests/sanity-lnet.sh b/lustre/tests/sanity-lnet.sh index e3659f9..5271e0f 100755 --- a/lustre/tests/sanity-lnet.sh +++ b/lustre/tests/sanity-lnet.sh @@ -2292,6 +2292,9 @@ test_214() { check_ni_status "$nid1" up check_ni_status "$nid2" up + do_lnetctl ping --source $nid2 $nid1 || + error "$LNETCTL ping --source $nid2 $nid1 failed" + echo "Set $FAKE_IF down" echo "ip link set dev $FAKE_IF down" ip link set dev $FAKE_IF down -- 1.8.3.1