cs: Cs_Insert cleanup

Author: yoursunny

container/cs/test_test.go

@@ -24,7 +24,6 @@ import (
 	"github.com/usnistgov/ndn-dpdk/ndn/ndntestenv"
 	"github.com/usnistgov/ndn-dpdk/ndni"
 	"github.com/usnistgov/ndn-dpdk/ndni/ndnitestenv"
-	"go4.org/must"
 )
 
 func TestMain(m *testing.M) {
@@ -42,7 +41,8 @@ var (
 )
 
 type Fixture struct {
-	t testing.TB
+	t       testing.TB
+	require *require.Assertions
 
 	Pcct       *pcct.Pcct
 	Cs         *cs.Cs
@@ -58,7 +58,7 @@ type Fixture struct {
 }
 
 func NewFixture(t testing.TB, cfg pcct.Config) (f *Fixture) {
-	f = &Fixture{t: t}
+	f = &Fixture{t: t, require: require.New(t)}
 
 	cfg.PcctCapacity = 4095
 	if cfg.CsMemoryCapacity == 0 {
@@ -70,46 +70,40 @@ func NewFixture(t testing.TB, cfg pcct.Config) (f *Fixture) {
 
 	var e error
 	f.Pcct, e = pcct.New(cfg, eal.NumaSocket{})
-	if e != nil {
-		panic(e)
-	}
+	f.require.NoError(e)
 	f.Cs = cs.FromPcct(f.Pcct)
 	f.Pit = pit.FromPcct(f.Pcct)
 
 	f.Fib, e = fib.New(fibdef.Config{Capacity: 1023}, []fib.LookupThread{&fibtestenv.LookupThread{}})
-	f.noError(e)
+	f.require.NoError(e)
 	placeholderName := ndn.ParseName("/75f3c2eb-6147-4030-afbc-585b3ce876a9")
 	e = f.Fib.Insert(makeFibEntry(placeholderName, nil, 9999))
-	f.noError(e)
+	f.require.NoError(e)
 	f.FibReplica = f.Fib.Replica(eal.NumaSocket{})
 	f.FibEntry = f.FibReplica.Lpm(placeholderName)
 
 	t.Cleanup(func() {
-		must.Close(f.Fib)
-		must.Close(f.Pcct)
+		f.require.NoError(f.Fib.Close())
+		f.require.NoError(f.Pcct.Close())
 		ealthread.AllocClear()
 	})
 	return f
 }
 
-func (f *Fixture) noError(e error) {
-	require.NoError(f.t, e)
-}
-
 // EnableDisk enables on-disk caching on a Malloc Bdev.
 func (f *Fixture) EnableDisk(nSlots int64) {
 	var e error
 	f.Bdev, e = bdev.NewMalloc((1 + nSlots) * 16)
-	f.noError(e)
+	f.require.NoError(e)
 	f.t.Cleanup(func() { f.Bdev.Close() })
 
 	f.SpdkTh, e = spdkenv.NewThread()
-	f.noError(e)
+	f.require.NoError(e)
 	f.t.Cleanup(func() { f.SpdkTh.Close() })
-	f.noError(ealthread.AllocLaunch(f.SpdkTh))
+	f.require.NoError(ealthread.AllocLaunch(f.SpdkTh))
 
 	f.DiskStore, e = disk.NewStore(f.Bdev, f.SpdkTh, 16, disk.StoreGetDataGo)
-	f.noError(e)
+	f.require.NoError(e)
 
 	min, max := f.DiskStore.SlotRange()
 	f.DiskAlloc = disk.NewAlloc(min, max, eal.NumaSocket{})
@@ -119,7 +113,7 @@ func (f *Fixture) EnableDisk(nSlots int64) {
 	})
 
 	e = f.Cs.SetDisk(f.DiskStore, f.DiskAlloc)
-	f.noError(e)
+	f.require.NoError(e)
 }
 
 // CountMpInUse returns number of in-use entries in PCCT's underlying mempool.
@@ -138,15 +132,13 @@ func (f *Fixture) Insert(interest *ndni.Packet, data *ndni.Packet) (isReplacing
 		data.Close()
 		return false
 	}
-	if pitEntry == nil {
-		panic("Pit.Insert failed")
-	}
+	f.require.NotNil(pitEntry, "Pit.Insert failed")
 
 	var pitFound pit.FindResult
 	for {
 		pitFound = f.Pit.FindByData(data, pitEntry.PitToken())
 		if len(pitFound.ListEntries()) == 0 {
-			panic("Pit.FindByData returned empty result")
+			f.require.Fail("Pit.FindByData returned empty result")
 		}
 
 		if pitFound.NeedDataDigest() {

csrc/pcct/cs-arc.c

@@ -31,6 +31,8 @@ const ptrdiff_t CsArc_ListOffsets_[] = {
 __attribute__((nonnull)) static void
 CsArc_MoveHandler(CsEntry* entry, CsListID src, CsListID dst, __rte_unused uintptr_t ctx) {
   switch (CsArc_MoveDir(src, dst)) {
+    case CsArc_MoveDirC(T1, Del):
+    case CsArc_MoveDirC(T2, Del):
     case CsArc_MoveDirC(T1, B1):
     case CsArc_MoveDirC(T2, B2):
       CsEntry_FreeData(entry);
@@ -40,7 +42,6 @@ CsArc_MoveHandler(CsEntry* entry, CsListID src, CsListID dst, __rte_unused uintp
     case CsArc_MoveDirC(B1, T2):
     case CsArc_MoveDirC(B2, T2):
     case CsArc_MoveDirC(B1, Del):
-    case CsArc_MoveDirC(T1, Del):
     case CsArc_MoveDirC(B2, Del):
       break;
     default:
@@ -175,7 +176,8 @@ CsArc_Add(CsArc* arc, CsEntry* entry) {
 
 void
 CsArc_Remove(CsArc* arc, CsEntry* entry) {
-  N_LOGD("Remove arc=%p cs-entry=%p", arc, entry);
+  N_LOGD("Remove arc=%p cs-entry=%p from=%d", arc, entry, (int)entry->arcList);
   CsList_Remove(CsArc_GetList(arc, entry->arcList), entry);
+  arc->moveCb((entry), entry->arcList, CslDirectDel, arc->moveCtx);
   entry->arcList = 0;
 }

csrc/pcct/cs-disk.c

@@ -49,6 +49,8 @@ void
 CsDisk_ArcMove(CsEntry* entry, CsListID src, CsListID dst, uintptr_t ctx) {
   Cs* cs = (Cs*)ctx;
   switch (CsArc_MoveDir(src, dst)) {
+    case CsArc_MoveDirC(T1, Del):
+    case CsArc_MoveDirC(T2, Del):
     case CsArc_MoveDirC(T1, B1):
       CsEntry_FreeData(entry);
       break;
@@ -65,7 +67,7 @@ CsDisk_ArcMove(CsEntry* entry, CsListID src, CsListID dst, uintptr_t ctx) {
     case CsArc_MoveDirC(T1, T2):
     case CsArc_MoveDirC(B1, T2):
     case CsArc_MoveDirC(B1, Del):
-    case CsArc_MoveDirC(T1, Del):
+      NDNDPDK_ASSERT(entry->kind != CsEntryDisk);
       break;
     default:
       NDNDPDK_ASSERT(false);

csrc/pcct/cs.c

@@ -37,35 +37,20 @@ CsEraseBatch_AddDirect(PcctEraseBatch* peb, CsEntry* entry) {
     CsEraseBatch_Append(peb, indirect, "indirect-dep");
   }
 
-  switch (entry->kind) {
-    case CsEntryNone:
-      break;
-    case CsEntryMemory:
-      CsEntry_FreeData(entry);
-      break;
-    default:
-      NDNDPDK_ASSERT(false);
-  }
+  NDNDPDK_ASSERT(entry->kind == CsEntryNone);
   CsEraseBatch_Append(peb, entry, "direct");
 }
 
 /** @brief Erase a CS entry including dependents. */
 __attribute__((nonnull)) static void
 Cs_EraseEntry(Cs* cs, CsEntry* entry) {
   PcctEraseBatch peb = PcctEraseBatch_New(Pcct_FromCs(cs));
-  switch (entry->kind) {
-    case CsEntryIndirect:
-      CsList_Remove(&cs->indirect, entry);
-      CsEraseBatch_AddIndirect(&peb, entry);
-      break;
-    case CsEntryDisk:
-      CsDisk_Delete(cs, entry);
-      // fallthrough
-    case CsEntryNone:
-    case CsEntryMemory:
-      CsArc_Remove(&cs->direct, entry);
-      CsEraseBatch_AddDirect(&peb, entry);
-      break;
+  if (entry->kind == CsEntryIndirect) {
+    CsList_Remove(&cs->indirect, entry);
+    CsEraseBatch_AddIndirect(&peb, entry);
+  } else {
+    CsArc_Remove(&cs->direct, entry);
+    CsEraseBatch_AddDirect(&peb, entry);
   }
   PcctEraseBatch_Finish(&peb);
 }
@@ -147,14 +132,15 @@ Cs_PutDirect(Cs* cs, Packet* npkt, PccEntry* pccEntry) {
       case CsEntryMemory:
         CsEntry_FreeData(entry);
         // fallthrough
+      case CsEntryDisk:
+        // diskSlot will be released by CsArc_Add that invokes CsDisk_ArcMove
+        // fallthrough
       case CsEntryNone:
         Cs_EraseImplicitDigestIndirect(cs, entry, data->name.length);
         break;
       case CsEntryIndirect:
         CsEntry_Disassoc(entry);
         break;
-      case CsEntryDisk: // TODO would this happen?
-        NDNDPDK_ASSERT(false);
     }
   } else {
     // insert direct entry
@@ -213,17 +199,6 @@ Cs_PutIndirect(Cs* cs, CsEntry* direct, PccEntry* pccEntry) {
     } else {
       // change direct entry to indirect entry
       CsArc_Remove(&cs->direct, entry);
-      switch (entry->kind) {
-        case CsEntryNone:
-          break;
-        case CsEntryMemory:
-          rte_pktmbuf_free(Packet_ToMbuf(entry->data));
-          entry->kind = CsEntryNone;
-          break;
-        case CsEntryDisk: // TODO would this happen?
-        default:
-          NDNDPDK_ASSERT(false);
-      }
     }
     N_LOGD("PutIndirect refresh cs=%p npkt=%p pcc-entry=%p cs-entry=%p", cs, direct, pccEntry,
            entry);
@@ -261,36 +236,27 @@ Cs_Insert(Cs* cs, Packet* npkt, PitFindResult pitFound) {
   PData* data = Packet_GetDataHdr(npkt);
   PccEntry* pccEntry = pitFound.entry;
   PInterest* interest = PitFindResult_GetInterest(pitFound);
-  CsEntry* direct = NULL;
-
-  // if Interest name differs from Data name, insert a direct entry elsewhere
-  if (unlikely(interest->name.nComps != data->name.nComps)) {
-    direct = Cs_InsertDirect(cs, npkt, interest);
-    if (unlikely(direct == NULL)) { // direct entry insertion failed
-      Pit_EraseSatisfied(&pcct->pit, pitFound);
-      rte_pktmbuf_free(pkt);
-      if (likely(!pccEntry->hasCsEntry)) {
-        Pcct_Erase(pcct, pccEntry);
-      }
-      return;
-    }
-    NULLize(npkt);
-    NULLize(pkt); // owned by direct entry
-  }
 
-  // delete PIT entries
-  Pit_EraseSatisfied(&pcct->pit, pitFound);
-  NULLize(interest);
+  if (interest->name.nComps == data->name.nComps) { // exact match, direct entry here
+    Pit_EraseSatisfied(&pcct->pit, pitFound);
+    NULLize(interest);
 
-  if (likely(direct == NULL)) {
-    // put direct CS entry at pccEntry
-    direct = Cs_PutDirect(cs, npkt, pccEntry);
-    // alloc-err cannot happen because PccSlots are freed from PIT entries
-    NDNDPDK_ASSERT(direct != NULL); // TODO this may happen now
-  } else {
-    // put indirect CS entry at pccEntry
+    CsEntry* direct = Cs_PutDirect(cs, npkt, pccEntry);
+    if (unlikely(direct == NULL)) {
+      goto FAIL_DIRECT;
+    }
+  } else { // prefix match, indirect entry here, direct entry elsewhere
+    CsEntry* direct = Cs_InsertDirect(cs, npkt, interest);
+    Pit_EraseSatisfied(&pcct->pit, pitFound);
+    NULLize(interest);
+
+    if (unlikely(direct == NULL)) {
+      goto FAIL_DIRECT;
+    }
     Cs_PutIndirect(cs, direct, pccEntry);
   }
+  NULLize(npkt); // owned by direct entry
+  NULLize(pkt);
 
   // evict if over capacity
   if (unlikely(cs->indirect.count > cs->indirect.capacity)) {
@@ -299,6 +265,13 @@ Cs_Insert(Cs* cs, Packet* npkt, PitFindResult pitFound) {
   if (unlikely(cs->direct.Del.count >= CsEvictBulk)) {
     Cs_Evict(cs, &cs->direct.Del, "direct", Cs_EvictEntryDirect);
   }
+
+  return;
+FAIL_DIRECT:
+  rte_pktmbuf_free(pkt);
+  if (likely(!pccEntry->hasEntries)) {
+    Pcct_Erase(pcct, pccEntry);
+  }
 }
 
 CsEntry*