From 28b8f3d9296d632a37e36c68c6000dc0c5e96e5a Mon Sep 17 00:00:00 2001 From: Qian Yingjin Date: Wed, 28 Feb 2018 17:22:01 +0800 Subject: [PATCH] LU-10737 misc: Wrong checksum return value In the checksum calculation functions: tgt_checksum_niobuf and osc_checksum_bulk, it is wrongly taken the error return value of cfs_crypto_hash_init as the checksum value. This patch fixes the problem. Signed-off-by: Qian Yingjin Change-Id: I647c402deeab00ec5c6437423b0cab250b42c3e5 Reviewed-on: https://review.whamcloud.com/31448 Tested-by: Jenkins Reviewed-by: Dmitry Eremin Tested-by: Maloo Reviewed-by: James Simmons Reviewed-by: Li Xi Reviewed-by: Andreas Dilger --- lustre/osc/osc_request.c | 49 ++++++++++++++++++++++++++++----------------- lustre/target/tgt_handler.c | 32 +++++++++++++++++------------ 2 files changed, 50 insertions(+), 31 deletions(-) diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index 2969858..925542b 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -1027,11 +1027,11 @@ static inline int can_merge_pages(struct brw_page *p1, struct brw_page *p2) return (p1->off + p1->count == p2->off); } -static u32 osc_checksum_bulk(int nob, size_t pg_count, +static int osc_checksum_bulk(int nob, size_t pg_count, struct brw_page **pga, int opc, - enum cksum_types cksum_type) + enum cksum_types cksum_type, + u32 *cksum) { - u32 cksum; int i = 0; struct cfs_crypto_hash_desc *hdesc; unsigned int bufsize; @@ -1070,15 +1070,15 @@ static u32 osc_checksum_bulk(int nob, size_t pg_count, i++; } - bufsize = sizeof(cksum); - cfs_crypto_hash_final(hdesc, (unsigned char *)&cksum, &bufsize); + bufsize = sizeof(*cksum); + cfs_crypto_hash_final(hdesc, (unsigned char *)cksum, &bufsize); /* For sending we only compute the wrong checksum instead * of corrupting the data so it is still correct on a redo */ if (opc == OST_WRITE && OBD_FAIL_CHECK(OBD_FAIL_OSC_CHECKSUM_SEND)) - cksum++; + (*cksum)++; - return cksum; + return 0; } static int @@ -1283,12 +1283,18 @@ no_bulk: body->oa.o_flags |= cksum_type_pack(cksum_type); body->oa.o_valid |= OBD_MD_FLCKSUM | OBD_MD_FLFLAGS; - body->oa.o_cksum = osc_checksum_bulk(requested_nob, - page_count, pga, - OST_WRITE, - cksum_type); + + rc = osc_checksum_bulk(requested_nob, page_count, + pga, OST_WRITE, cksum_type, + &body->oa.o_cksum); + if (rc < 0) { + CDEBUG(D_PAGE, "failed to checksum, rc = %d\n", + rc); + GOTO(out, rc); + } CDEBUG(D_PAGE, "checksum at write origin: %x\n", body->oa.o_cksum); + /* save this in 'oa', too, for later checking */ oa->o_valid |= OBD_MD_FLCKSUM | OBD_MD_FLFLAGS; oa->o_flags |= cksum_type_pack(cksum_type); @@ -1416,6 +1422,7 @@ check_write_checksum(struct obdo *oa, const struct lnet_process_id *peer, __u32 new_cksum; char *msg; enum cksum_types cksum_type; + int rc; if (server_cksum == client_cksum) { CDEBUG(D_PAGE, "checksum %x confirmed\n", client_cksum); @@ -1428,10 +1435,13 @@ check_write_checksum(struct obdo *oa, const struct lnet_process_id *peer, cksum_type = cksum_type_unpack(oa->o_valid & OBD_MD_FLFLAGS ? oa->o_flags : 0); - new_cksum = osc_checksum_bulk(aa->aa_requested_nob, aa->aa_page_count, - aa->aa_ppga, OST_WRITE, cksum_type); + rc = osc_checksum_bulk(aa->aa_requested_nob, aa->aa_page_count, + aa->aa_ppga, OST_WRITE, cksum_type, + &new_cksum); - if (cksum_type != cksum_type_unpack(aa->aa_oa->o_flags)) + if (rc < 0) + msg = "failed to calculate the client write checksum"; + else if (cksum_type != cksum_type_unpack(aa->aa_oa->o_flags)) msg = "the server did not use the checksum type specified in " "the original request - likely a protocol problem"; else if (new_cksum == server_cksum) @@ -1590,10 +1600,13 @@ static int osc_brw_fini_request(struct ptlrpc_request *req, int rc) cksum_type = cksum_type_unpack(body->oa.o_valid &OBD_MD_FLFLAGS? body->oa.o_flags : 0); - client_cksum = osc_checksum_bulk(rc, aa->aa_page_count, - aa->aa_ppga, OST_READ, - cksum_type); - + rc = osc_checksum_bulk(rc, aa->aa_page_count, aa->aa_ppga, + OST_READ, cksum_type, &client_cksum); + if (rc < 0) { + CDEBUG(D_PAGE, + "failed to calculate checksum, rc = %d\n", rc); + GOTO(out, rc); + } if (req->rq_bulk != NULL && peer->nid != req->rq_bulk->bd_sender) { via = " via "; diff --git a/lustre/target/tgt_handler.c b/lustre/target/tgt_handler.c index e7c96bb..745f92d 100644 --- a/lustre/target/tgt_handler.c +++ b/lustre/target/tgt_handler.c @@ -1712,15 +1712,15 @@ static void tgt_brw_unlock(struct obd_ioobj *obj, struct niobuf_remote *niob, tgt_extent_unlock(lh, mode); EXIT; } -static __u32 tgt_checksum_niobuf(struct lu_target *tgt, +static int tgt_checksum_niobuf(struct lu_target *tgt, struct niobuf_local *local_nb, int npages, - int opc, enum cksum_types cksum_type) + int opc, enum cksum_types cksum_type, + __u32 *cksum) { struct cfs_crypto_hash_desc *hdesc; unsigned int bufsize; int i, err; unsigned char cfs_alg = cksum_obd2cfs(cksum_type); - __u32 cksum; hdesc = cfs_crypto_hash_init(cfs_alg, NULL, 0); if (IS_ERR(hdesc)) { @@ -1795,10 +1795,10 @@ static __u32 tgt_checksum_niobuf(struct lu_target *tgt, } } - bufsize = sizeof(cksum); - err = cfs_crypto_hash_final(hdesc, (unsigned char *)&cksum, &bufsize); + bufsize = sizeof(*cksum); + err = cfs_crypto_hash_final(hdesc, (unsigned char *)cksum, &bufsize); - return cksum; + return 0; } char dbgcksum_file_name[PATH_MAX]; @@ -2084,9 +2084,12 @@ int tgt_brw_read(struct tgt_session_info *tsi) repbody->oa.o_flags = cksum_type_pack(cksum_type); repbody->oa.o_valid = OBD_MD_FLCKSUM | OBD_MD_FLFLAGS; - repbody->oa.o_cksum = tgt_checksum_niobuf(tsi->tsi_tgt, - local_nb, npages_read, - OST_READ, cksum_type); + rc = tgt_checksum_niobuf(tsi->tsi_tgt, local_nb, + npages_read, OST_READ, cksum_type, + &repbody->oa.o_cksum); + if (rc < 0) + GOTO(out_commitrw, rc); + CDEBUG(D_PAGE, "checksum at read origin: %x\n", repbody->oa.o_cksum); @@ -2426,10 +2429,12 @@ skip_transfer: repbody->oa.o_valid |= OBD_MD_FLCKSUM | OBD_MD_FLFLAGS; repbody->oa.o_flags &= ~OBD_FL_CKSUM_ALL; repbody->oa.o_flags |= cksum_type_pack(cksum_type); - repbody->oa.o_cksum = tgt_checksum_niobuf(tsi->tsi_tgt, - local_nb, npages, - OST_WRITE, - cksum_type); + rc = tgt_checksum_niobuf(tsi->tsi_tgt, local_nb, + npages, OST_WRITE, cksum_type, + &repbody->oa.o_cksum); + if (rc < 0) + GOTO(out_commitrw, rc); + cksum_counter++; if (unlikely(body->oa.o_cksum != repbody->oa.o_cksum)) { @@ -2448,6 +2453,7 @@ skip_transfer: } } +out_commitrw: /* Must commit after prep above in all cases */ rc = obd_commitrw(tsi->tsi_env, OBD_BRW_WRITE, exp, &repbody->oa, objcount, ioo, remote_nb, npages, local_nb, rc); -- 1.8.3.1