Feature #4528
openManagement thread
0%
Description
NFD currently implements its management protocol in the same thread as packet forwarding. As part of the management protocol implementation, the main thread is doing computationally intensive work, such as packet signing and signature verification, causing significant forwarding delays.
This issue moves NFD management to a separate thread, using the following approach:
- Data structure locking: Add a lightweight lock to NFD's data structures that are accessed by both forwarding and management. This includes FaceTable, FIB, StrategyChoice, and ContentStore.
- Management thread separation: Create a management thread. Connect management thread to forwarding through a Unix stream face.
Updated by Junxiao Shi over 6 years ago
- Blocks Feature #4529: Merge NFD-RIB into management thread added
Updated by Junxiao Shi over 6 years ago
- Related to Feature #4530: More efficient communication between threads added
Updated by Junxiao Shi over 6 years ago
- Blocked by Task #3565: Investigate management thread added
Updated by Davide Pesavento over 6 years ago
- Status changed from New to In Progress
Junxiao Shi wrote:
Connect management thread to forwarding through a Unix stream face.
I will not do this. It's faster to simply implement #4530 directly.
Updated by Junxiao Shi over 6 years ago
Connect management thread to forwarding through a Unix stream face.
I will not do this. It's faster to simply implement #4530 directly.
That’s fine. I wrote “Unix stream face” to reduce blocking relations at the cost of increasing total work amount.
Updated by Davide Pesavento over 6 years ago
With this change (and even more with #4529) the current source-level split into the three core
, daemon
, and rib
subdirs doesn't make sense anymore.
daemon/mgmt
and rib
should be together, and they should be separate from the rest of the modules that run on the main (forwarding) thread. Feel free to bikeshed how exactly this should look like.
Management unit tests should also be split from unit-tests-daemon
. In fact, I propose to go one step further and split unit-tests-daemon
into unit-tests-face
, unit-tests-fw
, unit-tests-table
, and unit-tests-mgmt
. After #4529, unit-tests-rib
can either be merged into unit-tests-mgmt
or be kept separate.
Updated by Junxiao Shi almost 6 years ago
Regarding codebase structure, I would say:
dp/face
: data plane, face systemdp/table
: data plane, tablesdp/fw
: data plane, forwardingcp/mgmt
: control plane, management protocolcp/rib
: control plane, RIB service
Updated by Davide Pesavento almost 6 years ago
There is an implicit desire to minimize the total number of changes, especially unnecessary ones. The current daemon/{face,fw,table}
subdir structure is perfectly fine, there is no reason to change it, so I will not touch it. I'm only going to change rib/
, daemon/mgmt
(if necessary), and probably some files in core/
. So your proposal does not work for me.
Also, what should happen to RibManager
? It's currently in rib/
but it's technically part of the management protocol.
Updated by Junxiao Shi almost 6 years ago
There is an implicit desire to minimize the total number of changes, especially unnecessary ones.
Minimizing diff size is explicitly a non-goal. See https://gerrit.named-data.net/#/c/NFD/+/332/1/daemon/fw/strategy.cpp@39
The current
daemon/{face,fw,table}
subdir structure is perfectly fine, there is no reason to change it, so I will not touch it.
Either way is fine. daemon
means "data plane". Just put that in daemon/README
.
Similarly, you may rename cp
to something better, but I'd suggest avoiding rib
or mgmt
, because control plane does not equal RIB or management.
Also, what should happen to
RibManager
? It's currently inrib/
but it's technically part of the management protocol.
It eventually should be cp/mgmt/rib-manager.hpp
.
This would require some object ownership changes in #4731.
Updated by Junxiao Shi almost 6 years ago
I don't understand what the problem is. You said "daemon/" is fine. Be more specific in your comments.
daemon
is fine as an alternative to dp
.
mgmt and rib belong to control plane and they need a separate folder other than daemon
.
Updated by Davide Pesavento almost 6 years ago
Not gonna do that. "daemon" means daemon, not dataplane. All code of nfd
binary goes in daemon/
subdir. As I already said, I will not change that.
Updated by Junxiao Shi almost 6 years ago
Due to this dispute, this issue needs to be discussed in next NFD call.
Updated by Alex Afanasyev over 5 years ago
I completely agree with Davide. Whatever control not control plane is the business of the daemon and not source code organization. The structure is already non-trivial to understand, I see absolutely no reason to make more changes than minimally necessary.
Updated by Junxiao Shi over 5 years ago
In https://gerrit.named-data.net/5328 several components in the RIB thread start to use getScheduler()
global function instead of the m_scheduler
member variable. I remember that 6th-ndn-hackathon report indicates that globals should be avoid in favor of passing io_service
and schedulers around. What has changed since then, that leads to a different design?
Updated by Davide Pesavento over 5 years ago
I may have found a way to do it without passing io_service
and Scheduler
around. Of course getting rid of all "globals" is still possible, but I'd rather not do it unless strictly necessary, I tried with Scheduler
and you need to change really a lot of code just to pass the reference around to the classes that need it.
Updated by Davide Pesavento over 5 years ago
And more importantly, self-learning required adding the getMainIoService
and getRibIoService
helpers, which are another way to do this. We did investigate this approach during that hackathon, but ultimately abandoned it because we thought it was not particularly elegant and expected it to be rejected by reviewers.
Updated by Junxiao Shi about 4 years ago
how much performance improvement would you expect from separating mgmt out -- 20%? 50%?
Benefits depend on frequency of management commands.
It will most likely be less than 20%.