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

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 bit width 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

@@ -380,6 +380,54 @@ def test_table_entry_lpm(self, input_, value, length):
 
         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")
         with self.assertRaisesRegex(UserError, "LPM don't care match"):

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