From c7764f397c1bbffa22d2a6b60f043709adfdc045 Mon Sep 17 00:00:00 2001
From: Xuehan Xu <xuxuehan@qianxin.com>
Date: Mon, 5 Feb 2024 13:41:17 +0800
Subject: [PATCH] crimson/os/seastore/transaction_manager: check checksums when
 loading logical extents

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>
---
 src/crimson/os/seastore/cached_extent.h       |  4 ++
 .../lba_manager/btree/btree_lba_manager.h     |  4 ++
 src/crimson/os/seastore/transaction_manager.h | 60 +++++++++++++++----
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h
index 6fc430d039a..a9b69e24846 100644
--- a/src/crimson/os/seastore/cached_extent.h
+++ b/src/crimson/os/seastore/cached_extent.h
@@ -1067,6 +1067,10 @@ public:
   virtual key_t get_intermediate_key() const { return min_max_t<key_t>::null; }
   virtual key_t get_intermediate_base() const { return min_max_t<key_t>::null; }
   virtual extent_len_t get_intermediate_length() const { return 0; }
+  virtual uint32_t get_checksum() const {
+    ceph_abort("impossible");
+    return 0;
+  }
   // The start offset of the pin, must be 0 if the pin is not indirect
   virtual extent_len_t get_intermediate_offset() const {
     return std::numeric_limits<extent_len_t>::max();
diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
index 85ffe3b805c..d1e0428b9ca 100644
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.h
@@ -141,6 +141,10 @@ public:
     return get_map_val().refcount > 1;
   }
 
+  uint32_t get_checksum() const final {
+    return get_map_val().checksum;
+  }
+
 protected:
   std::unique_ptr<BtreeNodeMapping<laddr_t, paddr_t>> _duplicate(
     op_context_t<laddr_t> ctx) const final {
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 94b6bd5d2dd..b74c02b8949 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -864,18 +864,35 @@ private:
       pref.is_indirect() ?
 	pref.get_intermediate_length() :
 	pref.get_length(),
-      [pin=std::move(pin)]
+      [&pref]
       (T &extent) mutable {
 	assert(!extent.has_laddr());
 	assert(!extent.has_been_invalidated());
-	assert(!pin->has_been_invalidated());
-	assert(pin->get_parent());
-	pin->link_child(&extent);
-	extent.maybe_set_intermediate_laddr(*pin);
+	assert(!pref.has_been_invalidated());
+	assert(pref.get_parent());
+	pref.link_child(&extent);
+	extent.maybe_set_intermediate_laddr(pref);
       }
-    ).si_then([FNAME, &t](auto ref) mutable -> ret {
+    ).si_then([FNAME, &t, pin=std::move(pin)](auto ref) mutable -> ret {
       SUBTRACET(seastore_tm, "got extent -- {}", t, *ref);
       assert(ref->is_fully_loaded());
+      bool inconsistent = false;
+      if (pin->is_indirect()) {
+	inconsistent = (pin->get_checksum() != 0);
+      } else {
+	inconsistent = !(pin->get_checksum() == 0 || // TODO: remapped extents may
+						     // not have recorded chksums
+			 pin->get_checksum() == ref->get_crc32c());
+      }
+      if (unlikely(inconsistent)) {
+	SUBERRORT(seastore_tm,
+	  "extent checksum inconsistent, recorded: {}, actual: {}, {}",
+	  t,
+	  pin->get_checksum(),
+	  crc,
+	  *ref);
+	ceph_abort();
+      }
       return pin_to_extent_ret<T>(
 	interruptible::ready_future_marker{},
 	std::move(ref));
@@ -906,19 +923,36 @@ private:
       pref.is_indirect() ?
 	pref.get_intermediate_length() :
 	pref.get_length(),
-      [pin=std::move(pin)](CachedExtent &extent) mutable {
+      [&pref](CachedExtent &extent) mutable {
 	auto &lextent = static_cast<LogicalCachedExtent&>(extent);
 	assert(!lextent.has_laddr());
 	assert(!lextent.has_been_invalidated());
-	assert(!pin->has_been_invalidated());
-	assert(pin->get_parent());
-	assert(!pin->get_parent()->is_pending());
-	pin->link_child(&lextent);
-	lextent.maybe_set_intermediate_laddr(*pin);
+	assert(!pref.has_been_invalidated());
+	assert(pref.get_parent());
+	assert(!pref.get_parent()->is_pending());
+	pref.link_child(&lextent);
+	lextent.maybe_set_intermediate_laddr(pref);
       }
-    ).si_then([FNAME, &t](auto ref) {
+    ).si_then([FNAME, &t, pin=std::move(pin)](auto ref) {
       SUBTRACET(seastore_tm, "got extent -- {}", t, *ref);
       assert(ref->is_fully_loaded());
+      bool inconsistent = false;
+      if (pin->is_indirect()) {
+	inconsistent = (pin->get_checksum() != 0);
+      } else {
+	inconsistent = !(pin->get_checksum() == 0 || // TODO: remapped extents may
+						     // not have recorded chksums
+			 pin->get_checksum() == ref->get_crc32c());
+      }
+      if (unlikely(inconsistent)) {
+	SUBERRORT(seastore_tm,
+	  "extent checksum inconsistent, recorded: {}, actual: {}, {}",
+	  t,
+	  pin->get_checksum(),
+	  crc,
+	  *ref);
+	ceph_abort();
+      }
       return pin_to_extent_by_type_ret(
 	interruptible::ready_future_marker{},
 	std::move(ref->template cast<LogicalCachedExtent>()));
-- 
2.39.5