vppinfra: fix clib_bitmap_will_expand() result inversion

Pool's pool_put_will_expand() calls clib_bitmap_will_expand(),
so every put except ones that leads to free_bitmap reallocation
will get false positive results and vice versa.

Unfortunatelly there's no related test and existing bitmap
tests are failing silently with false positive result as well.

Fortunatelly neither clib_bitmap_will_expand() nor
pool_put_will_expand() are being used by current vpp codebase.

Type: fix
Signed-off-by: Vladislav Grishenko <themiron@yandex-team.ru>
Change-Id: Id5bb900cf6a1b1002d37670f5c415c74165b5421

diff --git a/src/plugins/unittest/bitmap_test.c b/src/plugins/unittest/bitmap_test.c
index f01b8bb..eb232bd 100644 (file)
--- a/src/plugins/unittest/bitmap_test.c
+++ b/src/plugins/unittest/bitmap_test.c
@@ -12,6 +12,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+#include <stdbool.h>
 #include <vlib/vlib.h>
 #include <vppinfra/bitmap.h>
 
@@ -51,6 +52,35 @@ done:
   return error;
 }
 
+static clib_error_t *
+check_bitmap_will_expand (const char *test_name, uword **bm, uword index,
+                         bool expected_will_expand)
+{
+  uword max_bytes = vec_max_bytes (*bm);
+  bool result;
+
+  result = clib_bitmap_will_expand (*bm, index);
+  if (result != expected_will_expand)
+    {
+      return clib_error_create (
+       "%s failed, wrong "
+       "bitmap's expansion before set (%u != %u expected)",
+       test_name, result, expected_will_expand);
+    }
+
+  *bm = clib_bitmap_set (*bm, index, 1);
+  result = vec_max_bytes (*bm) > max_bytes;
+  if (result != expected_will_expand)
+    {
+      return clib_error_create (
+       "%s failed, wrong "
+       "bitmap's expansion after set (%u != %u expected)",
+       test_name, result, expected_will_expand);
+    }
+
+  return 0;
+}
+
 static clib_error_t *
 test_bitmap_command_fn (vlib_main_t * vm,
                        unformat_input_t * input, vlib_cli_command_t * cmd)
@@ -58,6 +88,7 @@ test_bitmap_command_fn (vlib_main_t * vm,
   clib_error_t *error = 0;
   uword *bm = 0;
   uword *bm2 = 0;
+  uword *bm3 = 0;
   uword *dup = 0;
 
   /*  bm should look like:
@@ -162,9 +193,28 @@ test_bitmap_command_fn (vlib_main_t * vm,
   if (error != 0)
     goto done;
 
+  error = check_bitmap_will_expand ("clib_bitmap_will_expand 1", &bm, 0, 0);
+  if (error != 0)
+    goto done;
+
+  error = check_bitmap_will_expand ("clib_bitmap_will_expand 2", &bm,
+                                   vec_max_len (bm) * BITS (uword) - 1, 0);
+  if (error != 0)
+    goto done;
+
+  error = check_bitmap_will_expand ("clib_bitmap_will_expand 3", &bm,
+                                   vec_max_len (bm) * BITS (uword), 1);
+  if (error != 0)
+    goto done;
+
+  error = check_bitmap_will_expand ("clib_bitmap_will_expand 4", &bm3, 0, 1);
+  if (error != 0)
+    goto done;
+
 done:
   vec_free (bm);
   vec_free (bm2);
+  vec_free (bm3);
   vec_free (dup);
 
   return error;

diff --git a/src/vppinfra/bitmap.h b/src/vppinfra/bitmap.h
index d579afd..abb1405 100644 (file)
--- a/src/vppinfra/bitmap.h
+++ b/src/vppinfra/bitmap.h
@@ -208,7 +208,7 @@ clib_bitmap_set (uword * ai, uword i, uword value)
 always_inline u8
 clib_bitmap_will_expand (uword *ai, uword i)
 {
-  return (i / BITS (ai[0])) < vec_max_len (ai);
+  return (i / BITS (ai[0])) >= vec_max_len (ai);
 }
 
 /** Gets the ith bit value from a bitmap

diff --git a/test/asf/test_vppinfra.py b/test/asf/test_vppinfra.py
index 9151eb1..4b49628 100644 (file)
--- a/test/asf/test_vppinfra.py
+++ b/test/asf/test_vppinfra.py
@@ -8,8 +8,6 @@ from asfframework import VppTestCase, VppTestRunner
 class TestVppinfra(VppTestCase):
     """Vppinfra Unit Test Cases"""
 
-    vpp_worker_count = 1
-
     @classmethod
     def setUpClass(cls):
         super(TestVppinfra, cls).setUpClass()
@@ -25,16 +23,15 @@ class TestVppinfra(VppTestCase):
         super(TestVppinfra, self).tearDown()
 
     def test_bitmap_unittest(self):
-        """Bitmap Code Coverage Test"""
+        """Bitmap unit tests"""
+
         cmds = ["test bitmap"]
 
         for cmd in cmds:
-            r = self.vapi.cli_return_response(cmd)
-            if r.retval != 0:
-                if hasattr(r, "reply"):
-                    self.logger.info(cmd + " FAIL reply " + r.reply)
-                else:
-                    self.logger.info(cmd + " FAIL retval " + str(r.retval))
+            error = self.vapi.cli(cmd)
+            if error:
+                self.logger.critical(error)
+                self.assertNotIn("failed", error)
 
 
 if __name__ == "__main__":