Merge pull request ceph#65940 from Tom-Sollers/fixing_blaum_roth_for_ec_profiles

Mon: Add health warning for blaum-roth EC profile when w+1 is not prime

Author: Tom-Sollers

doc/rados/operations/health-checks.rst

@@ -1900,3 +1900,21 @@ To disable the debug mode, run the following command:
 .. prompt:: bash $
 
    ceph dashboard debug disable
+
+BLAUM_ROTH_W_IS_NOT_PRIME
+_________________________
+
+An EC pool is using the ``blaum_roth`` technique and ``w + 1`` is not a prime number. 
+This can result in data corruption.
+
+To check the list of Erasure Code Profiles use the command:
+
+.. prompt:: bash $
+   
+   ceph osd erasure-code-profile ls
+
+Then to check the ``w`` value for a particular profile use the command:
+
+.. prompt:: bash $
+
+   ceph osd erasure-code-profile get <name of profile>

qa/standalone/erasure-code/test-erasure-code-plugins.sh

@@ -115,4 +115,29 @@ function TEST_ec_profile_warning() {
     teardown $dir || return 1
 }
 
+function TEST_ec_profile_blaum_roth_warning() {
+    local dir=$1
+
+    setup $dir || return 1
+    run_mon $dir a || return 1
+    run_mgr $dir x || return 1
+    for id in $(seq 0 2) ; do
+        run_osd $dir $id || return 1
+    done
+    create_rbd_pool || return 1
+    wait_for_clean || return 1
+
+    echo "Starting test for blaum-roth profile health warning"
+
+    #Check that the health warn for incorrect blaum-roth profiles is correct.
+    ceph osd erasure-code-profile set prof-${plugin} plugin=jerasure k=3 m=1 technique=blaum_roth w=7 --yes-i-really-mean-it --force
+    CEPH_ARGS='' ceph --admin-daemon $(get_asok_path mon.a) log flush || return 1
+    sleep 10
+    grep -F "1 or more EC profiles have a w value such that w+1 is not prime. This can result in data corruption" $dir/mon.a.log || return 1
+    grep -F "w+1=8 for the EC profile prof-${plugin} is not prime and could lead to data corruption" $dir/mon.a.log || return 1
+
+    teardown $dir || return 1
+    return 0
+}
+
 main test-erasure-code-plugins "$@"

src/erasure-code/jerasure/ErasureCodeJerasure.cc

@@ -707,14 +707,14 @@ void ErasureCodeJerasureLiberation::prepare()
 // ErasureCodeJerasureBlaumRoth
 //
 bool ErasureCodeJerasureBlaumRoth::check_w(ostream *ss) const
-{
+{ 
   // back in Firefly, w = 7 was the default and produced usable
   // chunks. Tolerate this value for backward compatibility.
   if (w == 7)
     return true;
   if (w <= 2 || !is_prime(w+1)) {
     *ss <<  "w=" << w << " must be greater than two and "
-	<< "w+1 must be prime" << std::endl;
+	  << "w+1 must be prime" << std::endl;
     return false;
   } else {
     return true;

src/erasure-code/jerasure/ErasureCodeJerasure.h

@@ -305,6 +305,7 @@ class ErasureCodeJerasureBlaumRoth : public ErasureCodeJerasureLiberation {
   ErasureCodeJerasureBlaumRoth() :
     ErasureCodeJerasureLiberation("blaum_roth")
   {
+    DEFAULT_W = "6";
   }
 
   bool check_w(std::ostream *ss) const override;

src/mon/HealthMonitor.cc

@@ -27,6 +27,8 @@
 #include "mon/Monitor.h"
 #include "mon/HealthMonitor.h"
 #include "mon/OSDMonitor.h"
+#include "osd/OSDMap.h"
+
 
 #include "messages/MMonCommand.h"
 #include "messages/MMonHealthChecks.h"
@@ -749,6 +751,9 @@ bool HealthMonitor::check_leader_health()
   // STRETCH MODE
   check_mon_crush_loc_stretch_mode(&next);
 
+  //CHECK_ERASURE_CODE_PROFILE
+  check_erasure_code_profiles(&next);
+
   if (next != leader_checks) {
     changed = true;
     leader_checks = next;
@@ -1169,3 +1174,34 @@ void HealthMonitor::check_netsplit(health_check_map_t *checks, std::set<std::str
     d.detail.swap(details);
   }
 }
+
+void HealthMonitor::check_erasure_code_profiles(health_check_map_t *checks)
+{
+  list<string> details;
+  
+  //This is a loop that will go through all the erasure code profiles 
+  for (auto& erasure_code_profile : mon.osdmon()->osdmap.get_erasure_code_profiles()) {
+    dout(20) << "check_erasure_code_profiles" << "checking" << erasure_code_profile << dendl;
+
+    //This will look at the erasure code profiles technique is blaum_roth and will check that the w key exists 
+      if (erasure_code_profile.second.at("technique") == "blaum_roth" && 
+      erasure_code_profile.second.count("w") == 1) {
+        //Read the w value from the profile and convert it to an int 
+        int w = std::stoi(erasure_code_profile.second.at("w"));
+
+        if (!mon.is_prime(w + 1)) {
+          ostringstream ds;
+          ds << "w+1="<< w+1 << " for the EC profile " << erasure_code_profile.first 
+            << " is not prime and could lead to data corruption";
+          details.push_back(ds.str());
+      }
+    }
+  }
+  if (!details.empty()) {
+            ostringstream ss;
+            ss << "1 or more EC profiles have a w value such that w+1 is not prime."
+              << " This can result in data corruption";
+            auto &d = checks->add("BLAUM_ROTH_W_IS_NOT_PRIME", HEALTH_WARN, ss.str(), details.size());
+            d.detail.swap(details);
+        }
+}

src/mon/HealthMonitor.h

@@ -73,6 +73,7 @@ class HealthMonitor : public PaxosService
   void check_for_clock_skew(health_check_map_t *checks);
   void check_mon_crush_loc_stretch_mode(health_check_map_t *checks);
   void check_if_msgr2_enabled(health_check_map_t *checks);
+  void check_erasure_code_profiles(health_check_map_t *checks);
   void check_netsplit(health_check_map_t *checks, std::set<std::string> &mons_down);
   bool check_leader_health();
   bool check_member_health();

src/mon/Monitor.cc

@@ -7083,3 +7083,19 @@ void Monitor::disconnect_disallowed_stretch_sessions()
     session_stretch_allowed(*j, blank);
   }
 }
+
+// A utility function that will check to see if the given value is prime using a set of the first 55 prime numbers
+bool Monitor::is_prime(int value) {
+  int prime55[] = {
+    2,3,5,7,11,13,17,19,23,29,31,37,41,43,47,53,59,61,67,71,
+    73,79,83,89,97,101,103,107,109,113,127,131,137,139,149,
+    151,157,163,167,173,179,
+    181,191,193,197,199,211,223,227,229,233,239,241,251,257
+  };
+
+  for (int i: prime55)
+    if (value == i) 
+      return true;
+  
+  return false;
+}

src/mon/Monitor.h

@@ -283,6 +283,8 @@ class Monitor : public Dispatcher,
   bool is_degraded_stretch_mode() { return degraded_stretch_mode; }
   bool is_recovering_stretch_mode() { return recovering_stretch_mode; }
 
+  bool is_prime(int value); // A utility funtion that returns true if value is prime 
+
   /**
    * This set of functions maintains the in-memory stretch state
    * and sets up transitions of the map states by calling in to

src/mon/OSDMonitor.cc

@@ -11668,6 +11668,43 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
       err = -EINVAL;
       goto reply_no_propose;
     }
+
+    bool force_no_fake = false;
+	  cmd_getval(cmdmap, "yes_i_really_mean_it", force_no_fake);
+
+
+
+    //This is the start of the validation for the w value in a blaum_roth profile
+    //this will search the Profile map, which contains the values for the parameters given in the command, for the technique parameter
+    if (auto found = profile_map.find("technique"); found != profile_map.end()) {
+
+      //if the technique parameter is found then save the value of it
+      string technique = found->second;
+
+      
+      //then search the profile map again for the w value, which doesnt have to be specified, and if it is found and the technique used is blaum-roth then check that the w value is correct.
+      if (found = profile_map.find("w"); technique == "blaum_roth" && found != profile_map.end()) {
+        int w = std::stoi(found->second);
+        
+        //checks if w+1 is not prime
+        if (w <= 2 || !mon.is_prime(w + 1)) {
+
+          if (force ^ force_no_fake) {
+            err = -EPERM;
+            ss << "Creating a blaum-roth erasure code profile with a w+1 value that is not prime is dangerious,"
+              << " as it can cause data corruption."
+              << " You need to use both --yes-i-really-mean-it and --force flags." << std::endl;
+            goto reply_no_propose;
+          } else if (!force && !force_no_fake) {
+            ss << "erasure-code-profile: " << profile_map 
+              << " must use a w value such that w+1 is prime and w is greater than 2." << std::endl;
+            err = -EINVAL;
+            goto reply_no_propose;
+          }
+        }
+      }
+    }
+
     string plugin = profile_map["plugin"];
 
     if (pending_inc.has_erasure_code_profile(name)) {
@@ -11689,8 +11726,7 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
 	  err = 0;
 	  goto reply_no_propose;
 	}
-	bool force_no_fake = false;
-	cmd_getval(cmdmap, "yes_i_really_mean_it", force_no_fake);
+
 	if (!force) {
 	  err = -EPERM;
 	  ss << "will not override erasure code profile " << name