From d1509ff2ca29b2ac35a773ecb31523f92a1f06c6 Mon Sep 17 00:00:00 2001 From: Serguei Smirnov Date: Sat, 27 Jan 2024 12:17:34 -0800 Subject: [PATCH] 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. Lustre-change: https://review.whamcloud.com/53843 Lustre-commit: TBD (from 3360e892750d1bf4f2b7ceab60d9a637b3e649ad) Test-Parameters: testlist=sanity-lnet 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/ex/lustre-release/+/53846 Tested-by: jenkins Reviewed-by: Frank Sehr --- lnet/include/lnet/lib-lnet.h | 1 + lnet/lnet/lib-ptl.c | 66 ++++++++++++++++++++++++++++++++++++++------ lustre/tests/sanity.sh | 17 ++++++++++++ 3 files changed, 75 insertions(+), 9 deletions(-) mode change 100644 => 100755 lnet/lnet/lib-ptl.c diff --git a/lnet/include/lnet/lib-lnet.h b/lnet/include/lnet/lib-lnet.h index b23cf32..c2223c6 100644 --- a/lnet/include/lnet/lib-lnet.h +++ b/lnet/include/lnet/lib-lnet.h @@ -39,6 +39,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 old mode 100644 new mode 100755 index abdd6ae..1032ea7 --- a/lnet/lnet/lib-ptl.c +++ b/lnet/lnet/lib-ptl.c @@ -153,19 +153,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 (me->me_match_id.nid != LNET_NID_ANY && - 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 ((me->me_match_id.nid != LNET_NID_ANY && + 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_id2str(info->mi_id), + info->mi_mbits, + libcfs_nid2str(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 but NID mismatch %s rejected (different peer %pK != %pK)\n", + libcfs_id2str(info->mi_id), + info->mi_mbits, + libcfs_nid2str(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 but PID mismatch %s rejected\n", + libcfs_id2str(info->mi_id), info->mi_mbits, + libcfs_id2str(me->me_match_id)); + return LNET_MATCHMD_NONE; + } + } else { + /* there were no bits to match, reject on NID/PID mismatch */ + if (me->me_match_id.nid != LNET_NID_ANY && + 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 34cfb93..23d25c7 100755 --- a/lustre/tests/sanity.sh +++ b/lustre/tests/sanity.sh @@ -25320,6 +25320,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.14.0.114) )) || skip "Need OST version at least 2.14.0.114" -- 1.8.3.1