From fee0b2425a1c24b21150004e73c7d5c590c4a50e Mon Sep 17 00:00:00 2001
From: Antonin Bas <antonin@barefootnetworks.com>
Date: Thu, 8 Feb 2018 20:45:23 -0800
Subject: [PATCH] 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.
---
 src/bm_sim/match_tables.cpp |  5 ++++
 tests/test_tables.cpp       | 53 +++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/src/bm_sim/match_tables.cpp b/src/bm_sim/match_tables.cpp
index 83012a1f7..99932ebbd 100644
--- 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
index 230a59a70..d5d535ff0 100644
--- 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;