Fix member / group ref counting when used as default entries

The ref count was being incremented, which means we could end up with dangling default entries.
p4lang · Mar 14, 2018 · fee0b24 · fee0b24
1 parent 48991b4
commit fee0b24
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 0 deletions.
diff --git a/src/bm_sim/match_tables.cpp b/src/bm_sim/match_tables.cpp
@@ -776,8 +776,10 @@ MatchTableIndirect::set_default_member(mbr_hdl_t mbr) {
     if (!action_profile->is_valid_mbr(mbr)) {
       rc = MatchErrorCode::INVALID_MBR_HANDLE;
     } else {
+      if (default_set) action_profile->ref_count_decrease(default_index);
       default_index = IndirectIndex::make_mbr_index(mbr);
       default_set = true;
+      action_profile->ref_count_increase(default_index);
     }
   }
 
@@ -1004,8 +1006,11 @@ MatchTableIndirectWS::set_default_group(grp_hdl_t grp) {
     if (!action_profile->is_valid_grp(grp)) {
       rc = MatchErrorCode::INVALID_GRP_HANDLE;
     } else {
+      if (default_set) action_profile->ref_count_decrease(default_index);
       default_index = IndirectIndex::make_grp_index(grp);
       default_set = true;
+      action_profile->ref_count_increase(default_index);
+      default_set = true;
     }
   }
 

diff --git a/tests/test_tables.cpp b/tests/test_tables.cpp
@@ -1152,6 +1152,31 @@ TEST_F(TableIndirect, DeleteMember) {
   ASSERT_EQ(rc, MatchErrorCode::SUCCESS);
 }
 
+TEST_F(TableIndirect, DefaultMember) {
+  MatchErrorCode rc;
+  mbr_hdl_t mbr, mbr_2;
+
+  auto pkt = get_pkt(64);
+
+  rc = add_member(0xaba, &mbr);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = table->set_default_member(mbr);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = action_profile.delete_member(mbr);
+  EXPECT_EQ(rc, MatchErrorCode::MBR_STILL_USED);
+
+  rc = add_member(0xabb, &mbr_2);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = table->set_default_member(mbr_2);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = action_profile.delete_member(mbr);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+}
+
 TEST_F(TableIndirect, ModifyEntry) {
   MatchErrorCode rc;
   mbr_hdl_t mbr_1, mbr_2;
@@ -1775,6 +1800,34 @@ TEST_F(TableIndirectWS, GetEntries) {
   ASSERT_EQ(0u, e2.time_since_hit_ms);
 }
 
+TEST_F(TableIndirectWS, DefaultGroup) {
+  MatchErrorCode rc;
+  grp_hdl_t grp;
+  mbr_hdl_t mbr;
+  unsigned int data = 666u;
+
+  auto pkt = get_pkt(64);
+
+  rc = action_profile.create_group(&grp);
+  EXPECT_EQ(MatchErrorCode::SUCCESS, rc);
+  rc = add_member(data, &mbr);
+  EXPECT_EQ(MatchErrorCode::SUCCESS, rc);
+  rc = action_profile.add_member_to_group(mbr, grp);
+  EXPECT_EQ(MatchErrorCode::SUCCESS, rc);
+
+  rc = table->set_default_group(grp);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = action_profile.delete_group(grp);
+  EXPECT_EQ(rc, MatchErrorCode::GRP_STILL_USED);
+
+  rc = table->set_default_member(mbr);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+
+  rc = action_profile.delete_group(grp);
+  EXPECT_EQ(rc, MatchErrorCode::SUCCESS);
+}
+
 TEST_F(TableIndirectWS, CustomGroupSelection) {
   using GroupSelectionIface = ActionProfile::GroupSelectionIface;
   using hash_t = ActionProfile::hash_t;