LPM Masking: Fix masking for fields with bitwidth non divisable by 8.

When prefix and/or bitwidth are not aligend to size of a byte, we must make sure the mask is applied correctly to all bits in field.
Until now, the logic expected to find maximum of one byte that needs special masking, and that masking is relative to the "beginning" (MSB bits) of the bytes.
This caused issues with fields that are smaller than a byte.
Another issue, some fields and prefixed might need some bits of a the "first" and "last" byte in the prefix to be masked, for example:
Field bitwidth = 20
Prefix length = 16
Mask should be = 0b11111111111111110000
Since we are representing in byte arrays, in bytes we get -> 00001111 11111111 11110000  (24 bits, 3 bytes)

Old implmentation disregarded the first 2 bytes and only masked the last byte (0b11110000).

Both issues are fixed in this commit, by this flow:
1. Utilizing prefix byte array, which is aligned to number of bytes needed to represent bitwidth in bytes.
2. Creating a mask for entire field, according to prefix length.
3. Applying it to all bytes in prefix length byte array, except those who are fully masked (i.e. the ones in the middle, if exist).

Issue: #146
Signed-off-by: Dor Akerman <[email protected]>

Author: dorakerman

p4runtime_sh/shell.py

@@ -440,19 +440,36 @@ def _sanitize_and_convert_mf_lpm(self, prefix, length, field_info):
 
         barray = bytearray(prefix)
         transformed = False
-        r = length % 8
-        byte_mask = 0xff & ((0xff << (8 - r)))
-        if barray[first_byte_masked] & byte_mask != barray[first_byte_masked]:
-            transformed = True
-            barray[first_byte_masked] = barray[first_byte_masked] & byte_mask
-
-        for i in range(first_byte_masked + 1, len(prefix)):
-            if barray[i] != 0:
+        # When prefix mask and field bithwidth are not aligned to the size of a
+        # byte, we need to make sure mask is applied to the all bytes in prefix
+        # that should be masked.
+        # Therefore, we will create a byte array mask for all the bits in the
+        # field and apply it to entire prefix, zeroing out the bits that should
+        # not be part of the prefix. We can skip bytes that are fully inside
+        # the mask.
+        mask = ((1 << length) - 1)   # Create mask for the prefix length.
+        mask = mask << (field_info.bitwidth - length)  # Shift left to the correct position.
+        nbytes = (field_info.bitwidth + 7) // 8
+        bytes_mask = bytearray(mask.to_bytes(nbytes, byteorder='big'))
+
+        # Prefix len is aligned to num of byte needed to represent bitwidth in parsing stage.
+        if len(bytes_mask) != len(prefix):  # Should not happen, safety check.
+            raise UserError("Invalid prefix length")
+
+        idxs_to_apply_mask = list(range(first_byte_masked, len(bytes_mask)))
+        if first_byte_masked > 0:
+            idxs_to_apply_mask.insert(0, 0)  # Always apply mask to first byte.
+
+        transformed = False
+        for i in idxs_to_apply_mask:
+            if barray[i] & bytes_mask[i] != barray[i]:
                 transformed = True
-                barray[i] = 0
+                barray[i] = barray[i] & bytes_mask[i]
+
         if transformed:
             _print("LPM value was transformed to conform to the P4Runtime spec "
                    "(trailing bits must be unset)")
+
         mf.lpm.value = bytes(bytes_utils.make_canonical_if_option_set(barray))
         return mf
 

p4runtime_sh/test.py

@@ -379,6 +379,54 @@ def test_table_entry_lpm(self, input_, value, length):
             p4runtime_pb2.Update.INSERT, P4RuntimeEntity.table_entry, expected_entry)
 
         self.servicer.Write.assert_called_once_with(ProtoCmp(expected_req), ANY)
+    
+    @nose2.tools.params(
+      ("0xfaafe/16", "\\x0f\\xaa\\xf0", 16, "0x3/1", "\\x02", 1),
+      ("0xfaafe/20", "\\x0f\\xaa\\xfe", 20, "0x3/2", "\\x03", 2),
+      ("0xfaafe/8", "\\x0f\\xa0\\x00", 8, "0x1/1", "\\x00", 1),
+      ("0xfaafe/1", "\\x08\\x00\\x00", 1, "0x1/2", "\\x01", 2))
+    def test_table_entry_lpm_bitwidth_not_multiply_of_8(
+      self, input_20, value_20, length_20, input_2, value_2, length_2
+    ):
+        te = sh.TableEntry("LpmTwo")(action="actionA")
+        te.match["header_test.field20"] = input_20
+        te.match["header_test.field2"] = input_2
+        te.action["param"] = "aa:bb:cc:dd:ee:ff"
+        te.insert()
+
+        # Cannot use format here because it would require escaping all braces,
+        # which would make wiriting tests much more annoying
+        expected_entry = """
+table_id: 33567647
+match {
+  field_id: 1
+  lpm {
+    value: "%s"
+    prefix_len: %s
+  }
+}
+match {
+  field_id: 2
+  lpm {
+    value: "%s"
+    prefix_len: %s
+  }
+}
+action {
+  action {
+    action_id: 16783703
+    params {
+      param_id: 1
+      value: "\\xaa\\xbb\\xcc\\xdd\\xee\\xff"
+    }
+  }
+}
+""" % (value_20, length_20, value_2, length_2)
+
+        expected_req = self.make_write_request(
+            p4runtime_pb2.Update.INSERT, P4RuntimeEntity.table_entry, expected_entry)
+
+        self.servicer.Write.assert_called_once_with(ProtoCmp(expected_req), ANY)
 
     def test_table_entry_lpm_dont_care(self):
         te = sh.TableEntry("LpmOne")

p4runtime_sh/testdata/unittest.p4info.pb.txt

@@ -50,6 +50,34 @@ tables {
   }
   size: 512
 }
+tables {
+  preamble {
+    id: 33567647
+    name: "LpmTwo"
+    alias: "LpmTwo"
+  }
+  match_fields {
+    id: 1
+    name: "header_test.field20"
+    bitwidth: 20
+    match_type: LPM
+  }
+  match_fields {
+    id: 2
+    name: "header_test.field2"
+    bitwidth: 2
+    match_type: LPM
+  }
+  action_refs {
+    id: 16783703
+  }
+  action_refs {
+    id: 16800567
+    annotations: "@defaultonly"
+    scope: DEFAULT_ONLY
+  }
+  size: 512
+}
 tables {
   preamble {
     id: 33584148