From: Serguei Smirnov Date: Sat, 27 Jan 2024 20:17:34 +0000 (-0800) Subject: LU-17476 lnet: prefer to use bits only to match ME X-Git-Tag: 2.15.62~141 X-Git-Url: https://git.whamcloud.com/?a=commitdiff_plain;h=0b61b7d6d7940f67b75db2f4747169478512dd09;p=fs%2Flustre-release.git LU-17476 lnet: prefer to use bits only to match ME In some cases, it has been observed that a reply will arrive at the portal with the correct match bits, but is dropped by lnet_parse_put(). This appears to happen with LNet Multi-Rail peers, each having two separate NIDs. If a reply arrives with matchbits available and matching, but the NIDs don't match, confirm the match if the NIDs are found to belong to the same peer. This will only happen in cases where the reply would be dropped entirely, causing hundreds of seconds of delay until the RPC is resent, so the extra overhead of checking for a peer match before dropping the request is only in the error path and minimal compared to the alternative. Add CFS_FAIL_CHECK() for exercising the match NIDs code. That is in a hot codepath, but CFS_FAIL_CHECK() is marked unlikely() and this check is in the error case and _should_ only be hit when the message would have been dropped anyway, so it seems unlikely to impact performance in any meaningful way. Test-Parameters: testlist=sanity env=ONLY=350,ONLY_REPEAT=10 Signed-off-by: Serguei Smirnov Signed-off-by: Andreas Dilger Change-Id: I10e1a2142539ddf5dabc26ce962cec1f2cfcf3db Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/53843 Reviewed-by: Cyril Bordage Reviewed-by: Oleg Drokin Reviewed-by: Frank Sehr Tested-by: jenkins Tested-by: Maloo --- diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h index 8043e2c..68a48ae 100644 --- a/lnet/include/lnet/lib-lnet.h +++ b/lnet/include/lnet/lib-lnet.h @@ -38,6 +38,7 @@ /* LNET has 0xeXXX */ #define CFS_FAIL_PTLRPC_OST_BULK_CB2 0xe000 +#define CFS_FAIL_MATCH_MD_NID 0xe001 #include diff --git a/lnet/lnet/lib-ptl.c b/lnet/lnet/lib-ptl.c index df065e2..68dd1cc 100644 --- a/lnet/lnet/lib-ptl.c +++ b/lnet/lnet/lib-ptl.c @@ -152,19 +152,67 @@ lnet_try_match_md(struct lnet_libmd *md, if ((md->md_options & info->mi_opc) == 0) return LNET_MATCHMD_NONE; - /* mismatched ME nid/pid? */ - if (!LNET_NID_IS_ANY(&me->me_match_id.nid) && - !nid_same(&me->me_match_id.nid, &info->mi_id.nid)) - return LNET_MATCHMD_NONE; - - if (me->me_match_id.pid != LNET_PID_ANY && - me->me_match_id.pid != info->mi_id.pid) - return LNET_MATCHMD_NONE; - /* mismatched ME matchbits? */ if (((me->me_match_bits ^ info->mi_mbits) & ~me->me_ignore_bits) != 0) return LNET_MATCHMD_NONE; + /* mismatched ME nid/pid? */ + if (me->me_match_bits & ~me->me_ignore_bits) { + /* try to accept match based on bits only */ + if ((!LNET_NID_IS_ANY(&me->me_match_id.nid) && + !nid_same(&me->me_match_id.nid, &info->mi_id.nid)) || + CFS_FAIL_CHECK(CFS_FAIL_MATCH_MD_NID)) { + struct lnet_peer *lp_me, *lp_peer; + + /* check if ME NID matches another NID of same peer */ + lp_me = lnet_find_peer(&me->me_match_id.nid); + lp_peer = lnet_find_peer(&info->mi_id.nid); + + if (lp_me && lp_peer && (lp_me == lp_peer)) { + /* Shouldn't happen, but better than dropping + * message entirely. Print warning so we know + * it happens, and something needs to be fixed. + */ + CWARN("message from %s matched %llu with NID mismatch %s accepted (same peer %pK)\n", + libcfs_idstr(&info->mi_id), + info->mi_mbits, + libcfs_nidstr(&me->me_match_id.nid), + lp_me); + lnet_peer_decref_locked(lp_me); + lnet_peer_decref_locked(lp_peer); + } else { + CNETERR("message from %s matched %llu with NID mismatch %s rejected (different peer %pK != %pK)\n", + libcfs_idstr(&info->mi_id), + info->mi_mbits, + libcfs_nidstr(&me->me_match_id.nid), + lp_me, lp_peer); + if (lp_me) + lnet_peer_decref_locked(lp_me); + if (lp_peer) + lnet_peer_decref_locked(lp_peer); + + return LNET_MATCHMD_NONE; + } + } + + if (me->me_match_id.pid != LNET_PID_ANY && + me->me_match_id.pid != info->mi_id.pid) { + CNETERR("message from %s matched %llu with PID mismatch %s rejected\n", + libcfs_idstr(&info->mi_id), info->mi_mbits, + libcfs_idstr(&me->me_match_id)); + return LNET_MATCHMD_NONE; + } + } else { + /* there were no bits to match, reject on nid/pid mismatch */ + if (!LNET_NID_IS_ANY(&me->me_match_id.nid) && + !nid_same(&me->me_match_id.nid, &info->mi_id.nid)) + return LNET_MATCHMD_NONE; + + if (me->me_match_id.pid != LNET_PID_ANY && + me->me_match_id.pid != info->mi_id.pid) + return LNET_MATCHMD_NONE; + } + /* Hurrah! This _is_ a match; check it out... */ if ((md->md_options & LNET_MD_MANAGE_REMOTE) == 0) diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh index 91a5010..e67d765 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -27335,6 +27335,23 @@ test_319() { } run_test 319 "lost lease lock on migrate error" +test_350() { + local mdts=$(comma_list $(mdts_nodes)) + + mkdir $DIR/$tdir || error "mkdir $DIR/$tdir failed" + stack_trap "rm -r $DIR/$tdir" + + #force 1/100 of replies to take "NID mismatch" codepath + #define CFS_FAIL_MATCH_MD_NID 0xe001 CFS_FAIL_SOME 0x10000000 + do_nodes $mdts $LCTL set_param fail_loc=0x1000e001 fail_val=100 + + while ls -lR $DIR/$tdir > /dev/null; do :; done & + stack_trap "killall -9 ls || killall -9 ls" + + cp -a /etc $DIR/$tdir || error "cp failed" +} +run_test 350 "force NID mismatch path to be exercised" + test_360() { (( $OST1_VERSION >= $(version_code 2.15.58.96) )) || skip "Need OST version at least 2.15.58.96"