Repository: randombit/botan
Pull request #2871 (2021-12-21 22:49:17)

Various ZFEC inprovements
* New encoding API
* Express SSE2 version using SIMD_4x32
* Remove useless callback param
* Rewrite some awkward code in the matrix inversion


Patch:
diff --git a/doc/api_ref/zfec.rst b/doc/api_ref/zfec.rst
index 33f20e3e5..57d2bf92f 100644
--- a/doc/api_ref/zfec.rst
+++ b/doc/api_ref/zfec.rst
@@ -11,9 +11,11 @@ such that any ``K`` of ``N`` shares is sufficient to recover the
 entire original input.
 
 .. note::
-   Specific to the ZFEC format, the first ``K`` generated shares
-   are identical to the original input data, followed by ``N-K``
-   shares of error correcting code.
+   Specific to the ZFEC format, the first ``K`` generated shares are
+   identical to the original input data, followed by ``N-K`` shares of
+   error correcting code. This is very different from threshold secret
+   sharing, where having fewer than ``K`` shares gives no information
+   about the original input.
 
 ``ZFEC`` requires that the input length be exactly divisible by ``K``;
 if needed define a padding scheme to pad your input to the necessary
@@ -26,16 +28,20 @@ size.
      Set up for encoding or decoding using parameters ``k`` and ``n``.
      Both must be less than 256, and ``k`` must be less than ``n``.
 
-  .. cpp:function:: void encode(const uint8_t input[], size_t size, \
+  .. cpp:function:: void encode_shares(const std::vector<const uint8_t>& shares, \
+         size_t share_size, \
          std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb) const
 
-     Encode ``input`` of total length ``size`` into ``N`` shares. The ``output_cb``
-     function will be called once for each share (in unspecified order).
+     Encode ``K`` shares in ``shares`` each of length ``share_size`` into ``N``
+     shares, also each of length ``share_size``. The ``output_cb`` function will
+     be called once for each output share (in some unspecified and possibly
+     non-deterministic order).
 
-     The parameters to ``output_cb`` are: the current share, the maximum share (this
-     will always be ``K``), and the value and length of the share.
+     The parameters to ``output_cb`` are: the share being output, the share
+     contents, and the length of the encoded share (which will always be
+     equal to ``share_size``).
 
-  .. cpp:function:: void decode(const std::map<size_t, const uint8_t*>& shares, \
+  .. cpp:function:: void decode_shares(const std::map<size_t, const uint8_t*>& shares, \
          size_t share_size, \
          std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb) const
 
@@ -43,5 +49,4 @@ size.
      of ``share_size`` bytes. The shares are identified by a small
      integer (between 0 and 255).
 
-     The parameters to ``output_cb`` are similar to that of ``encode``.
-
+     The parameters to ``output_cb`` are similar to that of ``encode_shares``.
diff --git a/src/cli/speed.cpp b/src/cli/speed.cpp
index 2c37d26e8..8de6bf5a3 100644
--- a/src/cli/speed.cpp
+++ b/src/cli/speed.cpp
@@ -2209,15 +2209,14 @@ class Speed final : public Command
 
          Botan::ZFEC zfec(k, n);
 
-         const size_t share_size = 1024;
+         const size_t share_size = 256 * 1024;
 
          std::vector<uint8_t> input(share_size * k);
          rng().randomize(input.data(), input.size());
 
          std::vector<uint8_t> output(share_size * n);
 
-         auto enc_fn = [&](size_t share, size_t /*max_share*/,
-                           const uint8_t buf[], size_t len)
+         auto enc_fn = [&](size_t share, const uint8_t buf[], size_t len)
             {
             std::memcpy(&output[share*share_size], buf, len);
             };
@@ -2252,16 +2251,23 @@ class Speed final : public Command
             shares.erase(shares.begin());
             }
 
-         auto dec_fn = [&](size_t /*share*/, size_t /*max_share*/,
-                           const uint8_t /*buf*/[], size_t /*len*/)
+         std::vector<uint8_t> recovered(share_size * k);
+
+         auto dec_fn = [&](size_t share, const uint8_t buf[], size_t len)
             {
+            std::memcpy(&recovered[share * share_size], buf, len);
             };
 
          dec_timer->run_until_elapsed(msec, [&]() {
-            zfec.decode(shares, share_size, dec_fn);
+            zfec.decode_shares(shares, share_size, dec_fn);
          });
 
          record_result(dec_timer);
+
+         if(recovered != input)
+            {
+            error_output() << "ZFEC recovery failed\n";
+            }
          }
 
 #endif
diff --git a/src/cli/zfec.cpp b/src/cli/zfec.cpp
index 5c6fdd80f..ac31bc731 100644
--- a/src/cli/zfec.cpp
+++ b/src/cli/zfec.cpp
@@ -167,7 +167,7 @@ class FEC_Encode final : public Command
             input_data.push_back(0x00);
             }
 
-         auto encoder_fn = [&](size_t share, size_t N,
+         auto encoder_fn = [&](size_t share,
                                const uint8_t bits[], size_t len)
             {
             std::ostringstream output_fsname;
@@ -180,14 +180,14 @@ class FEC_Encode final : public Command
             else
                output_fsname << input;
 
-            output_fsname << "." << (share + 1) << "_" << N;
+            output_fsname << "." << (share + 1) << "_" << n;
 
             if(!suffix.empty())
                output_fsname << "." << suffix;
 
             std::ofstream output(output_fsname.str(), std::ios::binary);
 
-            FEC_Share fec_share(share, k, N, padding, bits, len);
+            FEC_Share fec_share(share, k, n, padding, bits, len);
             fec_share.serialize_to(*hash, output);
             };
 
@@ -283,7 +283,7 @@ class FEC_Decode final : public Command
 
          std::vector<uint8_t> decoded(share_size * k);
 
-         auto decoder_fn = [&](size_t share, size_t /*N*/,
+         auto decoder_fn = [&](size_t share,
                                const uint8_t bits[], size_t len)
             {
             std::memcpy(&decoded[share * share_size],
@@ -295,7 +295,7 @@ class FEC_Decode final : public Command
          for(auto& share: shares)
             share_ptrs[share.share_id()] = share.share_data();
 
-         fec.decode(share_ptrs, share_size, decoder_fn);
+         fec.decode_shares(share_ptrs, share_size, decoder_fn);
 
          auto decoded_digest = hash->process(
             decoded.data(), decoded.size() - (hash_len + padding));
diff --git a/src/lib/misc/zfec/zfec.cpp b/src/lib/misc/zfec/zfec.cpp
index 4a42963e5..ea4e946fb 100644
--- a/src/lib/misc/zfec/zfec.cpp
+++ b/src/lib/misc/zfec/zfec.cpp
@@ -117,6 +117,7 @@ const uint8_t* GF_MUL_TABLE(uint8_t y)
             {
             m_table.resize(256 * 256);
 
+            // x*0 = 0*y = 0 so we iterate over [1,255)
             for(size_t i = 1; i != 256; ++i)
                {
                for(size_t j = 1; j != 256; ++j)
@@ -147,28 +148,33 @@ void invert_matrix(uint8_t matrix[], size_t K)
    class pivot_searcher
       {
       public:
-         pivot_searcher(size_t K) : ipiv(K) {}
+         pivot_searcher(size_t K) : m_ipiv(K) {}
 
-         std::pair<size_t, size_t> operator()(size_t col, const uint8_t* matrix)
+         std::pair<size_t, size_t> operator()(size_t col, const uint8_t matrix[])
             {
-            const size_t K = ipiv.size();
+            /*
+            * Zeroing column 'col', look for a non-zero element.
+            * First try on the diagonal, if it fails, look elsewhere.
+            */
 
-            if(ipiv[col] == false && matrix[col*K + col] != 0)
+            const size_t K = m_ipiv.size();
+
+            if(m_ipiv[col] == false && matrix[col*K + col] != 0)
                {
-               ipiv[col] = true;
+               m_ipiv[col] = true;
                return std::make_pair(col, col);
                }
 
             for(size_t row = 0; row != K; ++row)
                {
-               if(ipiv[row])
+               if(m_ipiv[row])
                   continue;
 
                for(size_t i = 0; i != K; ++i)
                   {
-                  if(ipiv[i] == false && matrix[row*K + i] != 0)
+                  if(m_ipiv[i] == false && matrix[row*K + i] != 0)
                      {
-                     ipiv[i] = true;
+                     m_ipiv[i] = true;
                      return std::make_pair(row, i);
                      }
                   }
@@ -179,25 +185,19 @@ void invert_matrix(uint8_t matrix[], size_t K)
 
       private:
          // Marks elements already used as pivots
-         std::vector<bool> ipiv;
+         std::vector<bool> m_ipiv;
       };
 
    pivot_searcher pivot_search(K);
    std::vector<size_t> indxc(K);
    std::vector<size_t> indxr(K);
-   std::vector<uint8_t> id_row(K);
 
    for(size_t col = 0; col != K; ++col)
       {
-      /*
-      * Zeroing column 'col', look for a non-zero element.
-      * First try on the diagonal, if it fails, look elsewhere.
-      */
+      const auto icolrow = pivot_search(col, matrix);
 
-      std::pair<size_t, size_t> icolrow = pivot_search(col, matrix);
-
-      size_t icol = icolrow.first;
-      size_t irow = icolrow.second;
+      const size_t icol = icolrow.first;
+      const size_t irow = icolrow.second;
 
       /*
       * swap rows irow and icol, so afterwards the diagonal
@@ -213,17 +213,15 @@ void invert_matrix(uint8_t matrix[], size_t K)
       indxr[col] = irow;
       indxc[col] = icol;
       uint8_t* pivot_row = &matrix[icol*K];
-      uint8_t c = pivot_row[icol];
+      const uint8_t c = pivot_row[icol];
+      pivot_row[icol] = 1;
 
       if(c == 0)
          throw Invalid_Argument("ZFEC: singlar matrix");
 
       if(c != 1)
          {
-         pivot_row[icol] = 1;
-
          const uint8_t* mul_c = GF_MUL_TABLE(GF_INVERSE[c]);
-
          for(size_t i = 0; i != K; ++i)
             pivot_row[i] = mul_c[pivot_row[i]];
          }
@@ -231,32 +229,22 @@ void invert_matrix(uint8_t matrix[], size_t K)
       /*
       * From all rows, remove multiples of the selected row to zero
       * the relevant entry (in fact, the entry is not zero because we
-      * know it must be zero). Here, if we know that the pivot_row is
-      * the identity, we can optimize the addmul.
+      * know it must be zero).
       */
-      id_row[icol] = 1;
-      if(std::memcmp(pivot_row, &id_row[0], K) != 0)
+      for(size_t i = 0; i != K; ++i)
          {
-         uint8_t* p = matrix;
-
-         for(size_t i = 0; i != K; ++i)
+         if(i != icol)
             {
-            if(i != icol)
-               {
-               c = p[icol];
-               p[icol] = 0;
+            const uint8_t z = matrix[i*K + icol];
+            matrix[i*K + icol] = 0;
 
-               const uint8_t* GF_MUL_C = GF_MUL_TABLE(c);
-
-               // This is equivalent to addmul()
-               for(size_t j = 0; j != K; ++j)
-                  p[j] ^= GF_MUL_C[pivot_row[j]];
-               }
-            p += K;
+            // This is equivalent to addmul()
+            const uint8_t* mul_z = GF_MUL_TABLE(z);
+            for(size_t j = 0; j != K; ++j)
+               matrix[i*K + j] ^= mul_z[pivot_row[j]];
             }
          }
-      id_row[icol] = 0;
-      } /* done all columns */
+      }
 
    for(size_t i = 0; i != K; ++i)
       {
@@ -424,7 +412,7 @@ ZFEC::ZFEC(size_t K, size_t N) :
 
    /*
    * quick code to build systematic matrix: invert the top
-   * K*K vandermonde matrix, multiply right the bottom n-K rows
+   * K*K Vandermonde matrix, multiply right the bottom n-K rows
    * by the inverse, and construct the identity matrix at the top.
    */
    create_inverted_vdm(&temp_matrix[0], m_K);
@@ -441,16 +429,18 @@ ZFEC::ZFEC(size_t K, size_t N) :
    /*
    * computes C = AB where A is n*K, B is K*m, C is n*m
    */
-   for(size_t row = m_K*m_K; row != m_N*m_K; row += m_K)
+   for(size_t row = m_K; row != m_N; ++row)
       {
       for(size_t col = 0; col != m_K; ++col)
          {
-         const uint8_t* pa = &temp_matrix[row];
-         const uint8_t* pb = &temp_matrix[col];
          uint8_t acc = 0;
-         for(size_t i = 0; i < m_K; i++, pa++, pb += m_K)
-            acc ^= GF_MUL_TABLE(*pa)[*pb];
-         m_enc_matrix[row + col] = acc;
+         for(size_t i = 0; i != m_K; i++)
+            {
+            const uint8_t row_v = temp_matrix[row * m_K + i];
+            const uint8_t row_c = temp_matrix[col + m_K * i];
+            acc ^= GF_MUL_TABLE(row_v)[row_c];
+            }
+         m_enc_matrix[row * m_K + col] = acc;
          }
       }
    }
@@ -460,38 +450,57 @@ ZFEC::ZFEC(size_t K, size_t N) :
 */
 void ZFEC::encode(
    const uint8_t input[], size_t size,
-   std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb)
+   output_cb_t output_cb)
    const
    {
    if(size % m_K != 0)
       throw Invalid_Argument("ZFEC::encode: input must be multiple of K uint8_ts");
 
-   size_t block_size = size / m_K;
+   const size_t share_size = size / m_K;
 
+   std::vector<const uint8_t*> shares;
    for(size_t i = 0; i != m_K; ++i)
-      output_cb(i, m_N, input + i*block_size, block_size);
+      shares.push_back(input + i*share_size);
+
+   this->encode_shares(shares, share_size, output_cb);
+   }
+
+void ZFEC::encode_shares(
+   const std::vector<const uint8_t*>& shares,
+   size_t share_size,
+   output_cb_t output_cb)
+   const
+   {
+   if(shares.size() != m_K)
+      throw Invalid_Argument("ZFEC::encode_shares must provide K shares");
+
+   // The initial shares are just the original input shares
+   for(size_t i = 0; i != m_K; ++i)
+      output_cb(i, shares[i], share_size);
+
+   std::vector<uint8_t> fec_buf(share_size);
 
    for(size_t i = m_K; i != m_N; ++i)
       {
-      std::vector<uint8_t> fec_buf(block_size);
+      clear_mem(fec_buf.data(), fec_buf.size());
 
       for(size_t j = 0; j != m_K; ++j)
          {
-         addmul(&fec_buf[0], input + j*block_size,
-                m_enc_matrix[i*m_K+j], block_size);
+         addmul(&fec_buf[0], shares[j],
+                m_enc_matrix[i*m_K+j], share_size);
          }
 
-      output_cb(i, m_N, &fec_buf[0], fec_buf.size());
+      output_cb(i, &fec_buf[0], fec_buf.size());
       }
    }
 
 /*
 * ZFEC decoding routine
 */
-void ZFEC::decode(
+void ZFEC::decode_shares(
    const std::map<size_t, const uint8_t*>& shares,
    size_t share_size,
-   std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb) const
+   output_cb_t output_cb) const
    {
    /*
    Todo:
@@ -543,7 +552,7 @@ void ZFEC::decode(
       if(share_id < m_K)
          {
          decoding_matrix[i*(m_K+1)] = 1;
-         output_cb(share_id, m_K, share_data, share_size);
+         output_cb(share_id, share_data, share_size);
          }
       else // will decode after inverting matrix
          {
@@ -569,7 +578,7 @@ void ZFEC::decode(
             {
             addmul(&buf[0], sharesv[col], decoding_matrix[i*m_K + col], share_size);
             }
-         output_cb(i, m_K, &buf[0], share_size);
+         output_cb(i, &buf[0], share_size);
          }
       }
    }
diff --git a/src/lib/misc/zfec/zfec.h b/src/lib/misc/zfec/zfec.h
index 3ebbab693..9b22447bc 100644
--- a/src/lib/misc/zfec/zfec.h
+++ b/src/lib/misc/zfec/zfec.h
@@ -30,6 +30,8 @@ namespace Botan {
 class BOTAN_PUBLIC_API(3,0) ZFEC
    {
    public:
+      typedef std::function<void (size_t, const uint8_t[], size_t)> output_cb_t;
+
       /**
       * FEC constructor
       * @param K the number of shares needed for recovery
@@ -49,7 +51,18 @@ class BOTAN_PUBLIC_API(3,0) ZFEC
       */
       void encode(
          const uint8_t input[], size_t size,
-         std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb)
+         output_cb_t output_cb)
+         const;
+
+      /**
+      * @param shares exactly K shares of data to FEC
+      * @param share_size the length in bytes of each share
+      * @param output_cb the output callback
+      */
+      void encode_shares(
+         const std::vector<const uint8_t*>& shares,
+         size_t share_size,
+         output_cb_t output_cb)
          const;
 
       /**
@@ -57,10 +70,10 @@ class BOTAN_PUBLIC_API(3,0) ZFEC
       * @param share_size size in bytes of each share
       * @param output_cb the output callback
       */
-      void decode(
+      void decode_shares(
          const std::map<size_t, const uint8_t*>& shares,
          size_t share_size,
-         std::function<void (size_t, size_t, const uint8_t[], size_t)> output_cb)
+         output_cb_t output_cb)
          const;
 
    private:
diff --git a/src/lib/misc/zfec/zfec_sse2/zfec_sse2.cpp b/src/lib/misc/zfec/zfec_sse2/zfec_sse2.cpp
index de399915c..6622a681c 100644
--- a/src/lib/misc/zfec/zfec_sse2/zfec_sse2.cpp
+++ b/src/lib/misc/zfec/zfec_sse2/zfec_sse2.cpp
@@ -5,39 +5,51 @@
 */
 
 #include <botan/zfec.h>
+#include <botan/internal/simd_32.h>
 #include <immintrin.h>
 
 namespace Botan {
 
+namespace {
+
+inline SIMD_4x32 rshift_1_u8(SIMD_4x32 v)
+   {
+   return SIMD_4x32(_mm_add_epi8(v.raw(), v.raw()));
+   }
+
+inline SIMD_4x32 high_bit_set_u8(SIMD_4x32 v)
+   {
+   return SIMD_4x32(_mm_cmpgt_epi8(_mm_setzero_si128(), v.raw()));
+   }
+
+}
+
 BOTAN_FUNC_ISA("sse2")
 size_t ZFEC::addmul_sse2(uint8_t z[], const uint8_t x[], uint8_t y, size_t size)
    {
-   // we assume the caller has aligned z to 16 for us!
-
-   const __m128i polynomial = _mm_set1_epi8(0x1D);
-   const __m128i zero = _mm_setzero_si128();
+   const SIMD_4x32 polynomial = SIMD_4x32::splat_u8(0x1D);
 
    const size_t orig_size = size;
 
    // unrolled out to cache line size
    while(size >= 64)
       {
-      __m128i x_1 = _mm_loadu_si128(reinterpret_cast<const __m128i*>(x));
-      __m128i x_2 = _mm_loadu_si128(reinterpret_cast<const __m128i*>(x + 16));
-      __m128i x_3 = _mm_loadu_si128(reinterpret_cast<const __m128i*>(x + 32));
-      __m128i x_4 = _mm_loadu_si128(reinterpret_cast<const __m128i*>(x + 48));
+      SIMD_4x32 x_1 = SIMD_4x32::load_le(x);
+      SIMD_4x32 x_2 = SIMD_4x32::load_le(x + 16);
+      SIMD_4x32 x_3 = SIMD_4x32::load_le(x + 32);
+      SIMD_4x32 x_4 = SIMD_4x32::load_le(x + 48);
 
-      __m128i z_1 = _mm_load_si128(reinterpret_cast<const __m128i*>(z));
-      __m128i z_2 = _mm_load_si128(reinterpret_cast<const __m128i*>(z + 16));
-      __m128i z_3 = _mm_load_si128(reinterpret_cast<const __m128i*>(z + 32));
-      __m128i z_4 = _mm_load_si128(reinterpret_cast<const __m128i*>(z + 48));
+      SIMD_4x32 z_1 = SIMD_4x32::load_le(z);
+      SIMD_4x32 z_2 = SIMD_4x32::load_le(z + 16);
+      SIMD_4x32 z_3 = SIMD_4x32::load_le(z + 32);
+      SIMD_4x32 z_4 = SIMD_4x32::load_le(z + 48);
 
       if(y & 0x01)
          {
-         z_1 = _mm_xor_si128(z_1, x_1);
-         z_2 = _mm_xor_si128(z_2, x_2);
-         z_3 = _mm_xor_si128(z_3, x_3);
-         z_4 = _mm_xor_si128(z_4, x_4);
+         z_1 ^= x_1;
+         z_2 ^= x_2;
+         z_3 ^= x_3;
+         z_4 ^= x_4;
          }
 
       for(size_t j = 1; j != 8; ++j)
@@ -47,41 +59,35 @@ size_t ZFEC::addmul_sse2(uint8_t z[], const uint8_t x[], uint8_t y, size_t size)
          * depending on if the high bit of x_i is set or not.
          */
 
-         // flip operation?
-         __m128i mask_1 = _mm_cmpgt_epi8(zero, x_1);
-         __m128i mask_2 = _mm_cmpgt_epi8(zero, x_2);
-         __m128i mask_3 = _mm_cmpgt_epi8(zero, x_3);
-         __m128i mask_4 = _mm_cmpgt_epi8(zero, x_4);
+         const SIMD_4x32 mask_1(high_bit_set_u8(x_1));
+         const SIMD_4x32 mask_2(high_bit_set_u8(x_2));
+         const SIMD_4x32 mask_3(high_bit_set_u8(x_3));
+         const SIMD_4x32 mask_4(high_bit_set_u8(x_4));
 
          // x <<= 1
-         x_1 = _mm_add_epi8(x_1, x_1);
-         x_2 = _mm_add_epi8(x_2, x_2);
-         x_3 = _mm_add_epi8(x_3, x_3);
-         x_4 = _mm_add_epi8(x_4, x_4);
-
-         mask_1 = _mm_and_si128(mask_1, polynomial);
-         mask_2 = _mm_and_si128(mask_2, polynomial);
-         mask_3 = _mm_and_si128(mask_3, polynomial);
-         mask_4 = _mm_and_si128(mask_4, polynomial);
+         x_1 = rshift_1_u8(x_1);
+         x_2 = rshift_1_u8(x_2);
+         x_3 = rshift_1_u8(x_3);
+         x_4 = rshift_1_u8(x_4);
 
-         x_1 = _mm_xor_si128(x_1, mask_1);
-         x_2 = _mm_xor_si128(x_2, mask_2);
-         x_3 = _mm_xor_si128(x_3, mask_3);
-         x_4 = _mm_xor_si128(x_4, mask_4);
+         x_1 ^= mask_1 & polynomial;
+         x_2 ^= mask_2 & polynomial;
+         x_3 ^= mask_3 & polynomial;
+         x_4 ^= mask_4 & polynomial;
 
          if((y >> j) & 1)
             {
-            z_1 = _mm_xor_si128(z_1, x_1);
-            z_2 = _mm_xor_si128(z_2, x_2);
-            z_3 = _mm_xor_si128(z_3, x_3);
-            z_4 = _mm_xor_si128(z_4, x_4);
+            z_1 ^= x_1;
+            z_2 ^= x_2;
+            z_3 ^= x_3;
+            z_4 ^= x_4;
             }
          }
 
-      _mm_store_si128(reinterpret_cast<__m128i*>(z     ), z_1);
-      _mm_store_si128(reinterpret_cast<__m128i*>(z + 16), z_2);
-      _mm_store_si128(reinterpret_cast<__m128i*>(z + 32), z_3);
-      _mm_store_si128(reinterpret_cast<__m128i*>(z + 48), z_4);
+      z_1.store_le(z);
+      z_2.store_le(z + 16);
+      z_3.store_le(z + 32);
+      z_4.store_le(z + 48);
 
       x += 64;
       z += 64;
diff --git a/src/lib/misc/zfec/zfec_vperm/zfec_vperm.cpp b/src/lib/misc/zfec/zfec_vperm/zfec_vperm.cpp
index e420701c0..47fbece2d 100644
--- a/src/lib/misc/zfec/zfec_vperm/zfec_vperm.cpp
+++ b/src/lib/misc/zfec/zfec_vperm/zfec_vperm.cpp
@@ -537,13 +537,13 @@ alignas(256) const uint8_t GFTBL[256*32] = {
 0x00, 0xff, 0xe3, 0x1c, 0xdb, 0x24, 0x38, 0xc7, 0xab, 0x54, 0x48, 0xb7, 0x70, 0x8f, 0x93, 0x6c,
 0x00, 0x4b, 0x96, 0xdd, 0x31, 0x7a, 0xa7, 0xec, 0x62, 0x29, 0xf4, 0xbf, 0x53, 0x18, 0xc5, 0x8e };
 
-inline SIMD_4x32 BOTAN_FUNC_ISA(BOTAN_VPERM_ISA) table_lookup(SIMD_4x32 a, SIMD_4x32 b)
+inline SIMD_4x32 BOTAN_FUNC_ISA(BOTAN_VPERM_ISA) table_lookup(SIMD_4x32 t, SIMD_4x32 v)
    {
 #if defined(BOTAN_SIMD_USE_SSE2)
-   return SIMD_4x32(_mm_shuffle_epi8(a.raw(), b.raw()));
+   return SIMD_4x32(_mm_shuffle_epi8(t.raw(), v.raw()));
 #elif defined(BOTAN_SIMD_USE_NEON)
-   const uint8x16_t tbl = vreinterpretq_u8_u32(a.raw());
-   const uint8x16_t idx = vreinterpretq_u8_u32(b.raw());
+   const uint8x16_t tbl = vreinterpretq_u8_u32(t.raw());
+   const uint8x16_t idx = vreinterpretq_u8_u32(v.raw());
 
 #if defined(BOTAN_TARGET_ARCH_IS_ARM32)
    const uint8x8x2_t tbl2 = { vget_low_u8(tbl), vget_high_u8(tbl) };
@@ -564,8 +564,6 @@ inline SIMD_4x32 BOTAN_FUNC_ISA(BOTAN_VPERM_ISA) table_lookup(SIMD_4x32 a, SIMD_
 BOTAN_FUNC_ISA(BOTAN_VPERM_ISA)
 size_t ZFEC::addmul_vperm(uint8_t z[], const uint8_t x[], uint8_t y, size_t size)
    {
-   // we assume the caller has aligned z to 16 for us!
-
    const auto mask = SIMD_4x32::splat_u8(0x0F);
 
    // fetch the lookup tables for the given y


Test patch:
diff --git a/src/tests/test_zfec.cpp b/src/tests/test_zfec.cpp
index 1baca3b2d..f4b9b8335 100644
--- a/src/tests/test_zfec.cpp
+++ b/src/tests/test_zfec.cpp
@@ -58,7 +58,7 @@ class ZFEC_KAT final : public Text_Based_Test
 
          std::set<size_t> shares_encoded;
 
-         auto zfec_enc_fn = [&](size_t share, size_t max_share,
+         auto zfec_enc_fn = [&](size_t share,
                                 const uint8_t block[], size_t len)
             {
             if(shares_encoded.insert(share).second == false)
@@ -66,8 +66,7 @@ class ZFEC_KAT final : public Text_Based_Test
                result.test_failure("Encoding returned the same share twice");
                }
 
-            result.test_eq_sz("ZFEC enc max share", max_share, N);
-            result.test_lt("ZFEC enc share in range", share, max_share);
+            result.test_lt("ZFEC enc share in range", share, N);
 
             result.test_eq(zfec_impl.c_str(),
                            "share " + std::to_string(share),
@@ -83,7 +82,7 @@ class ZFEC_KAT final : public Text_Based_Test
          // First test full decoding:
          std::set<size_t> shares_decoded;
 
-         auto zfec_dec_fn = [&](size_t share, size_t max_share,
+         auto zfec_dec_fn = [&](size_t share,
                                 const uint8_t block[], size_t len)
             {
             if(shares_decoded.insert(share).second == false)
@@ -91,8 +90,7 @@ class ZFEC_KAT final : public Text_Based_Test
                result.test_failure("Decoding returned the same share twice");
                }
 
-            result.test_eq_sz("ZFEC dec max share", max_share, K);
-            result.test_lt("ZFEC dec share in range", share, max_share);
+            result.test_lt("ZFEC dec share in range", share, K);
 
             result.test_eq(zfec_impl.c_str(),
                            "share " + std::to_string(share),
@@ -100,7 +98,7 @@ class ZFEC_KAT final : public Text_Based_Test
                            &input[share * share_size], share_size);
             };
 
-         zfec.decode(shares, share_size, zfec_dec_fn);
+         zfec.decode_shares(shares, share_size, zfec_dec_fn);
 
          result.test_eq("Correct number of shares decoded",
                         shares_decoded.size(), K);
@@ -114,7 +112,7 @@ class ZFEC_KAT final : public Text_Based_Test
             shares.erase(idx);
             }
 
-         zfec.decode(shares, share_size, zfec_dec_fn);
+         zfec.decode_shares(shares, share_size, zfec_dec_fn);
 
          result.test_eq("Correct number of shares decoded",
                         shares_decoded.size(), K);