From 01a625b9dcd38c13eaef57dc252389c76b72536f Mon Sep 17 00:00:00 2001 From: Dor Akerman Date: Wed, 10 Sep 2025 12:09:47 +0300 Subject: [PATCH] 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: https://github.com/p4lang/p4runtime-shell/issues/146 Signed-off-by: Dor Akerman --- p4runtime_sh/shell.py | 35 ++++++++++---- p4runtime_sh/test.py | 48 ++++++++++++++++++++ p4runtime_sh/testdata/unittest.p4info.pb.txt | 28 ++++++++++++ 3 files changed, 102 insertions(+), 9 deletions(-) diff --git a/p4runtime_sh/shell.py b/p4runtime_sh/shell.py index e791743..a9b980a 100644 --- a/p4runtime_sh/shell.py +++ b/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 diff --git a/p4runtime_sh/test.py b/p4runtime_sh/test.py index f6d15ef..4261ab3 100644 --- a/p4runtime_sh/test.py +++ b/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"): diff --git a/p4runtime_sh/testdata/unittest.p4info.pb.txt b/p4runtime_sh/testdata/unittest.p4info.pb.txt index 6a3bda1..8db55f1 100644 --- a/p4runtime_sh/testdata/unittest.p4info.pb.txt +++ b/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