Added tests for validating LPM byte alignment when bitwidth not divisible by 8

proto/frontend/src/common.cpp

@@ -148,8 +148,9 @@ Code check_proto_bytestring(const std::string &str, size_t nbits) {
   return Code::OK;
 }
 
-bool check_prefix_trailing_zeros(const std::string &str, int pLen) {
-  size_t bitwidth = str.size() * 8;
+bool check_prefix_trailing_zeros(const std::string &str, int pLen,
+                                 size_t bitwidth) {
+  // Use field bitwidth, not storage size (str.size() * 8)
   // must be guaranteed by caller
   assert(pLen >= 0 && static_cast<size_t>(pLen) <= bitwidth);
   size_t trailing_zeros = bitwidth - pLen;

proto/frontend/src/common.h

@@ -164,7 +164,8 @@ std::string pi_port_to_bytestring(pi_port_t port, size_t num_bytes);
 
 Code check_proto_bytestring(const std::string &str, size_t nbits);
 
-bool check_prefix_trailing_zeros(const std::string &str, int pLen);
+bool check_prefix_trailing_zeros(const std::string &str, int pLen,
+                                  size_t bitwidth);
 
 std::string range_default_lo(size_t nbits);
 std::string range_default_hi(size_t nbits);

proto/frontend/src/device_mgr.cpp

@@ -2246,7 +2246,7 @@ class DeviceMgrImp {
           "omit match field instead of using a prefix length of 0");
     }
     // makes sure that value ends with zeros
-    if (!common::check_prefix_trailing_zeros(value, pLen)) {
+    if (!common::check_prefix_trailing_zeros(value, pLen, bitwidth)) {
       RETURN_ERROR_STATUS(
           Code::INVALID_ARGUMENT,
           "Invalid LPM value, incorrect number of trailing zeros");

proto/tests/test_proto_fe.cpp

@@ -3410,6 +3410,189 @@ TEST_F(LpmOneTest, DontCare) {
 
 #undef EXPECT_ONE_TABLE_ENTRY
 
+#define EXPECT_ONE_TABLE_ENTRY(response, expected_entry)                \
+  do {                                                                  \
+    const auto &entities = response.entities();                         \
+    ASSERT_EQ(1, entities.size());                                      \
+    EXPECT_PROTO_EQ(entities.Get(0).table_entry(), expected_entry);    \
+  } while (false)
+
+// Test class for LpmNonByte table with 20-bit field (non-byte-aligned)
+class LpmNonByteTest : public DeviceMgrTest {
+ protected:
+  LpmNonByteTest()
+      : LpmNonByteTest("LpmNonByte", "header_test.field20") { }
+
+  LpmNonByteTest(const std::string &t_name, const std::string &f_name)
+      : f_name(f_name) {
+    t_id = pi_p4info_table_id_from_name(p4info, t_name.c_str());
+    a_id = pi_p4info_action_id_from_name(p4info, "actionA");
+  }
+
+  p4v1::TableEntry make_entry(const boost::optional<std::string> &mf_v,
+                              int pLen,
+                              const std::string &param_v) {
+    p4v1::TableEntry table_entry;
+    table_entry.set_table_id(t_id);
+    if (mf_v.is_initialized()) {
+      auto mf = table_entry.add_match();
+      mf->set_field_id(pi_p4info_table_match_field_id_from_name(
+          p4info, t_id, f_name.c_str()));
+      auto mf_lpm = mf->mutable_lpm();
+      mf_lpm->set_value(*mf_v);
+      mf_lpm->set_prefix_len(pLen);
+    }
+    auto entry = table_entry.mutable_action();
+    auto action = entry->mutable_action();
+
+    action->set_action_id(a_id);
+    auto param = action->add_params();
+    param->set_param_id(
+        pi_p4info_action_param_id_from_name(p4info, a_id, "param"));
+    param->set_value(param_v);
+    return table_entry;
+  }
+
+  const std::string f_name;
+  pi_p4_id_t t_id;
+  pi_p4_id_t a_id;
+};
+
+// Test that validates byte alignment for 20-bit LPM field (non-byte-aligned)
+//
+// According to P4Runtime spec, a 20-bit field is stored in 3 bytes with MSB
+// padding:
+//   Byte 0 [MSB]: 0000_xxxx (4 leading zero bits for padding)
+//   Byte 1:       xxxx_xxxx (8 bits of field)
+//   Byte 2 [LSB]: xxxx_xxxx (8 bits of field)
+//
+// The check_prefix_trailing_zeros() function has been FIXED to use field
+// bitwidth instead of storage size when calculating required trailing zeros.
+TEST_F(LpmNonByteTest, NonByteAlignedField) {
+  // First verify the table and field exist
+  ASSERT_NE(t_id, PI_INVALID_ID) << "Table LpmNonByte not found in p4info";
+  ASSERT_NE(a_id, PI_INVALID_ID) << "Action actionA not found in p4info";
+
+  std::string adata(6, '\xcd');
+
+  // Test with prefix length 20 (full field width)
+  // With the fix, all 20 bits of the field can be used!
+  int pLen(20);
+
+  {  // Valid: all 20 bits of the field set (no trailing zeros needed)
+    // Binary: 0000_1111 1111_1111 1111_1111
+    // Represents value 0xFFFFF (all 20 bits set)
+    std::string mf("\x0f\xff\xff", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  {  // Invalid: MSB has too many bits set (violates 20-bit field width)
+    // Binary: 1111_1111 1111_1111 1111_1111
+    std::string mf("\xff\xff\xff", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _)).Times(0);
+    auto status = add_entry(&entry);
+    EXPECT_EQ(status, OneExpectedError(Code::INVALID_ARGUMENT));
+  }
+
+  // Test with prefix length 16
+  // Requires 20-16=4 trailing zeros (not 24-16=8)
+  pLen = 16;
+
+  {  // Valid: first 16 bits of field set, last 4 bits zero
+    // Binary: 0000_1111 1111_1111 1111_0000
+    std::string mf("\x0f\xff\xf0", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  {  // Invalid: not enough trailing zeros (has 0, needs 4)
+    // Binary: 0000_1111 1111_1111 1111_1111
+    std::string mf("\x0f\xff\xff", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _)).Times(0);
+    auto status = add_entry(&entry);
+    EXPECT_EQ(status, OneExpectedError(Code::INVALID_ARGUMENT));
+  }
+
+  {  // Valid: more trailing zeros than needed (has 8, needs 4) - fine for LPM
+    // Binary: 0000_1111 1111_1111 0000_0000
+    std::string mf("\x0f\xff\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  // Test with prefix length 12
+  // Requires 20-12=8 trailing zeros (not 24-12=12)
+  pLen = 12;
+
+  {  // Valid: first 12 bits of field set, last 8 bits zero
+    // Binary: 0000_1111 1111_0000 0000_0000
+    std::string mf("\x0f\xf0\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  {  // Valid: alternative pattern with exactly 8 trailing zeros
+    // Binary: 0000_1111 1100_0000 0000_0000
+    std::string mf("\x0f\xc0\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  {  // Invalid: not enough trailing zeros (has 4, needs 8)
+    // Binary: 0000_1111 1111_1111 0000_0000
+    std::string mf("\x0f\xff\xf0", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _)).Times(0);
+    auto status = add_entry(&entry);
+    EXPECT_EQ(status, OneExpectedError(Code::INVALID_ARGUMENT));
+  }
+
+  {  // Valid: more trailing zeros than needed (has 12, needs 8) - fine for LPM
+    // Binary: 0000_1111 1000_0000 0000_0000
+    std::string mf("\x0f\x80\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  // Test with prefix length 8 (exactly 1 byte)
+  pLen = 8;
+
+  {  // Valid: first 8 bits set, last 12 bits zero
+    // Binary: 0000_1111 1000_0000 0000_0000
+    std::string mf("\x0f\x80\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+
+  {  // Valid: different pattern with 12 trailing zeros
+    // Binary: 0000_1010 0000_0000 0000_0000
+    std::string mf("\x0a\x00\x00", 3);
+    auto entry = make_entry(mf, pLen, adata);
+    EXPECT_CALL(*mock, table_entry_add(t_id, _, _, _));
+    auto status = add_entry(&entry);
+    ASSERT_EQ(status.code(), Code::OK);
+  }
+}
+
+#undef EXPECT_ONE_TABLE_ENTRY
+
 class TernaryTwoTest : public DeviceMgrTest {
  protected:
   TernaryTwoTest() {

tests/testdata/unittest.p4

@@ -94,6 +94,15 @@ control ingress(inout headers_t hdr, inout metadata_t meta, inout standard_metad
         size = 512;
     }
 
+    @name(".LpmNonByte")
+    table LpmNonByte {
+        key = {
+            hdr.header_test.field20 : lpm;
+        }
+        actions = { actionA; }
+        size = 512;
+    }
+
     @name(".TernaryOne")
     table TernaryOne {
         key = {

tests/testdata/unittest.p4info.txt

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