Task #1397
closed
NFD ControlCommand client
100%
Description
Update nfd::Controller to use combined "NFD Management protocol" ControlCommand.
Updated by Junxiao Shi over 10 years ago
- Status changed from New to In Progress
- Assignee set to Junxiao Shi
- Start date set to 03/23/2014
Updated by Junxiao Shi over 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 40
- Estimated time changed from 3.00 h to 4.00 h
I8d18fb2af799631bb9b94fad72f44c496e154a07 implements nfd::ControlParameters.
FaceManagementOptions & FibManagementOptions & StrategyChoiceOptions are aliases of ControlParameters.
These aliases will be deleted when nfd::Controller and NFD finishes transition.
Next change of this task is to update nfd::Controller.
Updated by Anonymous over 10 years ago
(Below comments moved from gerrit to foster discussion)
Overall, I like the centralized parameter class. It has definitely helped clean up the NFD-side code, but I feel like we've lost some usability in the process. The NFD side is going to need to validate that it has received the correct parameters for its protocol and that they have sane values.
It would be nice if the NFD side could say "I expect these fields" and have them validated. Similarly, the parameters should be self-documenting for clients. If you wanted to implement a fib management client, you would need to look at the wiki documentation to determine the necessary parameters (arguably a feature :) ). At the same time, there is value in understanding what is needed/used by looking at the class definition.
Updated by Junxiao Shi over 10 years ago
- Status changed from Code review to In Progress
Updated by Junxiao Shi over 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 40 to 50
I need a design review on Ieca08f5db530c14d1cd16ddd7af17e4ffe6eb344 patchset 1.
Every command will have a ControlCommand subclass that has detailed validation of request and response.
Updated by Alex Afanasyev over 10 years ago
I just noticed. Why do we allow arbitrary faceId in enable|disable-local-control-feature (http://redmine.named-data.net/projects/nfd/wiki/FaceMgmt#Enable-a-LocalControlHeader-feature) ??
I would mandate this parameter to be 0 or the same as self FaceId. Otherwise, authorized client can do bad things to other apps.
Updated by Junxiao Shi over 10 years ago
Protocol should not have such a restriction.
It should be two different privileges to faces/enable-local-control or fib/add-nexthop on self and on other face.
Updated by Alex Afanasyev over 10 years ago
Why would you ever mess up with other applications??? For me this is conceptually wrong.
Updated by Junxiao Shi over 10 years ago
- Status changed from Code review to In Progress
Updated by Junxiao Shi over 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 50 to 70
Ieca08f5db530c14d1cd16ddd7af17e4ffe6eb344
ControlCommand subclasses for all commands are defined.
I also updated protocol:
faces/enable-local-control and faces/disable-local-control forbid FaceId field.
These two commands can only operate on requesting face.
Updated by Junxiao Shi over 10 years ago
- Status changed from Code review to In Progress
I'm updating nfd::Controller to take a ControlCommand subclass.
Updated by Junxiao Shi over 10 years ago
- Status changed from In Progress to Code review
- % Done changed from 70 to 90
I4106c167e15b7cf4951687b3d18c4807c334d502
ndn::nfd::Controller takes ControlCommand subclass and ControlParameters.
Updated by Alex Afanasyev over 10 years ago
- Status changed from Code review to Closed
- % Done changed from 90 to 100