Project

General

Profile

Feature #2993

nfdc: create permanent faces

Added by Junxiao Shi about 5 years ago. Updated about 5 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Tools
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:
1.50 h

Description

In nfdc create command, add -P command line option.

When -P is specified, the face is created with FacePersistency=permanent; otherwise, it's created with FacePersistency=persistent.

nfdc manpage should be updated as part of this issue.


Related issues

Related to NFD - Feature #2991: FaceManager: specify face persistencyClosedYukai Tu

Actions
Blocked by NFD - Feature #2990: FaceMgmt: specify face persistencyClosedJunxiao Shi

Actions
Blocks NFD - Feature #2994: UDP permanent face scenarioClosedHila Ben Abraham

Actions
#1

Updated by Junxiao Shi about 5 years ago

  • Blocked by Feature #2990: FaceMgmt: specify face persistency added
#2

Updated by Junxiao Shi about 5 years ago

  • Related to Feature #2991: FaceManager: specify face persistency added
#3

Updated by Junxiao Shi about 5 years ago

#4

Updated by Junxiao Shi about 5 years ago

  • Description updated (diff)

There's a design choice on whether to support -P option in nfdc create only, or in all commands that can create a face.

I think it's sufficient to support -P option in nfdc create only.

If an operator wants to use a permanent face, the face can be created with nfdc create, before executing other commands.
FaceUri may still be used in other commands, which would attempt to locate an existing face (which is always available because it's permanent) before trying to create a persistent face.

#5

Updated by Joao Pereira about 5 years ago

Isn't this issue blocked by 2991 instead of related to?

Because we will need the ControlParameter change to send the persistency attribute

#6

Updated by Junxiao Shi about 5 years ago

Answer to note-5:

No. As long as the protocol is defined, both the client side and the server side can be developed in parallel.

#7

Updated by Joao Pereira about 5 years ago

Maybe I miss understood what is needed for this feature.

Shouldn't the change to solve this issue be made in this function by adding the parameter of persistency to the ControlParameter object?

void
Nfdc::FaceIdFetcher::startFaceCreate(const ndn::util::FaceUri& canonicalUri)
{
  ControlParameters parameters;
  parameters.setUri(canonicalUri.toString());

  m_controller.start<FaceCreateCommand>(parameters,
                                        [this] (const ControlParameters& result) {
                                          succeed(result.getFaceId());
                                        },
                                        bind(&FaceIdFetcher::onFaceCreateError, this, _1, _2,
                                             "Face creation failed"));
}

Because if that is the solution the ControlParameter need to be integrated before starting

#8

Updated by Alex Afanasyev about 5 years ago

We just merged commit that adds persistency to the ControlParameters, so this task can start.

#9

Updated by Joao Pereira about 5 years ago

  • Assignee set to Joao Pereira

Ok, I will pick this up.

There are no UT for this tool. Should they be created or not?

#10

Updated by Joao Pereira about 5 years ago

  • % Done changed from 0 to 100

I made a change without UT. the link is:

http://gerrit.named-data.net/2421

#11

Updated by Alex Afanasyev about 5 years ago

  • Status changed from New to Code review
#12

Updated by Joao Pereira about 5 years ago

In Gerrit there is a comment saying that the management files. Is it the task #2991 that should change these files?

#13

Updated by Alex Afanasyev about 5 years ago

Joao, I don't have a strong preference to separate issues into different commits. However, I will not disagree with Junxiao's desire to keep them separate. One thing that makes the other issue more complex is a test case (I will try to help with this).

#14

Updated by Junxiao Shi about 5 years ago

  • Status changed from Code review to Closed
#15

Updated by Junxiao Shi about 5 years ago

does one need to first create a permanent face, and then register the route through the same face URI?

Yes. The operator can also use FaceId in the register command.

The order between create and register commands is arbitrary. A face implicited created by register command is persistent, and it can be upgraded to permanent by a create command with -P flag.

Any reason not to have a shortcut that lets the -P argument be passed to the register command too?

Implicit face creation is necessary for register command because if the operator has to create a persistent face then register a route, it's possible for the face to fail before the register command succeeds.

However, this is unnecessary for permanent faces, because a permanent face can never fail.

Adding -P to register command unnecessarily complicates logic.

#16

Updated by Junxiao Shi about 5 years ago

From Jeff Burke:

I'd offer that from the user perspective, generally, it'd be beneficial to have slightly more complicated internal logic than to complicate the process for the end user or require what seems to be different semantics in this potentially quite common case. (The semantics of being able to do "create" after "register" is a little confusing – it's more of an "upgrade" or "modify" at that point than create.)

In all of our practical deployments, we will switch to using permanent UDP faces to try to minimize any need for manual intervention or watchdogs to recreate the route after interruptions. So having a single step consistent with typical registration but with an additional argument is preferred to two commands.

Either way, this detail should be in the developer's guide, and you might consider a log message that confirms the "upgrade" of the face more explicitly. The following certainly says it "succeeded" but is a little opaque, because from a user's perspective, it wasn't "creating" the face, but changing it's type. Also, the log message has no mention/confirmation of its "permanent" status.

jdisplay1:ndn jburke$ nfdc create -P udp4://spurs.cs.ucla.edu
...
1442849017.332018 WARNING: [FaceTable] Trying to add existing face id=266 to the face table
Face creation succeeded: ControlParameters(FaceId: 266, Uri: udp4://131.179.196.46:6363, )

Perhaps at face creation, the log should output the same details that are in nfd-status?

faceid=266 remote=udp4://131.179.196.46:6363 local=udp4://0.0.0.0:6363 counters={in={839i 8482d 10582387B} out={109830i 442d 12639710B}} non-local permanent point-to-point
#17

Updated by Davide Pesavento about 5 years ago

From Jeff Burke:

I'd offer that from the user perspective, generally, it'd be beneficial to have slightly more complicated internal logic than to complicate the process for the end user or require what seems to be different semantics in this potentially quite common case. (The semantics of being able to do "create" after "register" is a little confusing – it's more of an "upgrade" or "modify" at that point than create.)

+1

Either way, this detail should be in the developer's guide, and you might consider a log message that confirms the "upgrade" of the face more explicitly. The following certainly says it "succeeded" but is a little opaque, because from a user's perspective, it wasn't "creating" the face, but changing it's type. Also, the log message has no mention/confirmation of its "permanent" status.

Yeah we can/should improve the logic, so that we don't try to re-add an existing face, but just change its persistency setting.

#18

Updated by Jeff Burke about 5 years ago

Are there any conditions where permanent faces should not attempt to recover transport failures? It appears that some cases are not handled. Two examples:
1) Change in IP address of the default NIC caused by, for example, changing WiFi networks.
2) Restart of remote NFD.

Seems like both of these should be recoverable, assuming that there is still IP connectivity to the remote hub (in the case of #1) and the remote NFD comes up properly (#2), but this hasn't been my experience recently.

If these should be recovered in the current version, I'll doublecheck and work on replicating the issue.

#19

Updated by Peter Gusev about 5 years ago

Davide Pesavento wrote:

From Jeff Burke:

I'd offer that from the user perspective, generally, it'd be beneficial to have slightly more complicated internal logic than to complicate the process for the end user or require what seems to be different semantics in this potentially quite common case. (The semantics of being able to do "create" after "register" is a little confusing – it's more of an "upgrade" or "modify" at that point than create.)

+1

Either way, this detail should be in the developer's guide, and you might consider a log message that confirms the "upgrade" of the face more explicitly. The following certainly says it "succeeded" but is a little opaque, because from a user's perspective, it wasn't "creating" the face, but changing it's type. Also, the log message has no mention/confirmation of its "permanent" status.

Yeah we can/should improve the logic, so that we don't try to re-add an existing face, but just change its persistency setting.

+1

Or maybe provide a way to create a permanent face when registering prefix?

#20

Updated by Davide Pesavento about 5 years ago

Peter Gusev wrote:

Or maybe provide a way to create a permanent face when registering prefix?

Yes. My +1 above was because (1) in general I agree with Jeff's sentiment, and (2) more specifically I'd support adding -P to the register command. I suggest filing a separate ticket for this feature.

#21

Updated by Davide Pesavento about 5 years ago

Jeff Burke wrote:

Are there any conditions where permanent faces should not attempt to recover transport failures? It appears that some cases are not handled. Two examples:

1) Change in IP address of the default NIC caused by, for example, changing WiFi networks.

This scenario is probably not handled (yet). Currently we re-initialize only the multicast faces when connectivity changes (i.e. interface or address added/removed/changed).

2) Restart of remote NFD.

This should be handled gracefully by the permanent face, but please note that the implementation of #2989 is "minimal" so we might have missed some cases.

If these should be recovered in the current version, I'll doublecheck and work on replicating the issue.

Yes please open a ticket for each issue.

Also available in: Atom PDF