There is no API to delete the routing table and free its memory

[bakinat]

Hi,
Thanks for this great piece of SW.
One issue we found was that there is no API to delete/free the table.
We wrote the following C++ wrapper to do the deletion/free.
It first traverse the tree and collects all entries into a vector. Then it deletes each vector element from the table. Finally, it deletes the default entry and the root elements.
Note that mTable is a pointer to rtTable.
Two questions:

1. Can you check the code and confirm we did the right thing?
2. Can you add such API (maybe based on our code) to the SW?
   Thanks,
   Barak

IpArtWrapper::~IpArtWrapper()
{
vector<routeEnt*> rEnts;

auto func = [](routeEnt* ent, void* p) {
    auto pv = (vector<routeEnt*>*)p;
    pv->push_back(ent);
};

rtArtWalkTable(mTable, mTable->root, 1, 1 << mTable->psi[0].sl, func, &rEnts);

for (auto ent : rEnts)
    assert(mTable->deletep(mTable, ent->dest, ent->plen));

routeEnt* defRoute = mTable->root[1].ent;
if (defRoute) {
    // delete default route manually, since rtArtWalkTable doesn't give it
    u8 dst[16] = {0};
    assert(mTable->deletep(mTable, dst, 0));
}

free(mTable->psi);
// mTable->pEnt is freed automatically when deleting routes
free(mTable->pDef);
free(mTable->pTbl);
free(mTable->pPcSt);
free(mTable->root + mTable->off);
free(mTable);

}

[hariguchi]

Dear Barak,

Thank you for your comment. You are right. I forgot to add the table deletion API function. Here is my answers:

1. It seems OK although I cannot find the definitions of the followings:
   • deletep(): Maybe the same as bool (*delete)(rtTable* p, u8* pDest, int plen) in the original?
   • vector rEnts: Maybe std::vector<routeEnt*> rEnts?
2. Yes, I will add two API functions. One is rtArtFlushRoutes(rtTable *pt). This function will delete all the routes in *pt. The other is rtArtDeleteTable(rtTable *pt). This function will call rtArtFlushRoutes(), then deallocate all memories inside *pt, then free `pt' itself (just like your code does in the last part of the wrapper.) It will be written in C.

Cordially yours,
Yoichi

[bakinat]

1. yes - we renamed delete with deletep for C++ compatibility. yes -
   vector is std::vector - we had to use a vector since its not possible to
   delete the entries while performing a walk.
2. This will be great. You will probably not need a vector as the Flush
   will take care of it.
   Thank you!

------ Original Message ------
From: "Yoichi Hariguchi" notifications@github.com
To: "hariguchi/art" art@noreply.github.com
Cc: "bakinat" barak.enat@gmail.com; "Author"
author@noreply.github.com
Sent: 17/08/2016 22:50:09
Subject: Re: [hariguchi/art] There is no API to delete the routing table
and free its memory (#2)

Dear Barak,

Thank you for your comment. You are right. I forgot to add the table
deletion API function. Here is my answers:

It seems OK although I cannot find the definitions of the followings:
deletep(): Maybe the same as "bool (delete)(rtTable p, u8* pDest, int
plen)" in the original?vector rEnts: Maybe "std::vector rEnts"?Yes, I
will add two API functions. One is "rtArtFlushRoutes(rtTable pt)." This
function will delete all the routes in pt.' The other is "rtArtDeleteTable(rtTable pt)." This function will call rtArtFlushRoutes()', then deallocate all memories insidept', then free pt' itsel (just like your code does in the last part of the wrapper.)
It will be written in C.
Cordially yours,
Yoichi

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[hariguchi]

Hi Barak,

I implemented:

1. Flush Routes: which deletes all the route entries
2. Delete Routing Table: which deletes all the rout entries, then delete the table itself and free its memory.

Please check README and the source code.

[bakinat]

Thank you very much!
So far from my testing it looks good!

[hariguchi]

My pleasure. Unfortunately, this version iterates the trie twice (like your implementation.) The first iteration collects all the routes; the second iteration deletes them. I tried to make a single iteration version, but it is challenging due to lack of back pointers between nodes. I think it is OK since flushing will not happen that often.