Feature #4016
closedProtocolFactory::createFace accepts LocalUri
100%
Description
In particular, the task is to change API of createFace in ProtocolFactory and related classes to
virtual void
createFace(const FaceUri& remoteUri,
const FaceUri& localUri,
ndn::nfd::FacePersistency persistency,
bool wantLocalFieldsEnabled,
const FaceCreatedCallback& onCreated,
const FaceCreationFailedCallback& onFailure) = 0;
Also, the relevant changes need to be added to management to extract and pass information from the create command.
Updated by Alex Afanasyev over 7 years ago
- Blocked by Feature #4015: Accept LocalUri in FaceCreateCommand added
Updated by Alex Afanasyev over 7 years ago
- Blocks Feature #4017: nfdc face create: LocalUri added
Updated by Junxiao Shi over 7 years ago
- Tracker changed from Task to Feature
- Subject changed from Extend FaceSystem to support LocalUri when creating faces to ProtocolFactory::createFace accepts LocalUri
- Estimated time set to 3.00 h
Updated by Junxiao Shi over 7 years ago
- Blocked by deleted (Feature #4015: Accept LocalUri in FaceCreateCommand)
Updated by Junxiao Shi over 7 years ago
- Related to Feature #4015: Accept LocalUri in FaceCreateCommand added
Updated by Junxiao Shi over 7 years ago
- Blocks deleted (Feature #4017: nfdc face create: LocalUri)
Updated by Eric Newberry over 7 years ago
- Status changed from New to In Progress
Updated by Eric Newberry over 7 years ago
- Status changed from In Progress to Code review
- % Done changed from 0 to 100
Updated by Davide Pesavento over 7 years ago
Two design questions on https://gerrit.named-data.net/3809
TcpFactory
andUdpFactory
ignore the LocalUri even if it's provided, i.e. they accept everything. Is that ok or do we want to log a warning? or throw an exception (thus failing the command)? The latter would allow clients to determine if LocalUri selection is actually supported by the forwarder they're talking to, without relying on version number checks. It would also help preserve backward compatibility when the factories will start checking if the provided LocalUri makes sense (commands that were accepted won't suddenly start failing on the new version).I guess we also need a way to tell the protocol factories whether the LocalUri was specified in the request or not. The current approach taken by Change 3809 is to pass a non-canonical FaceUri (
null://
). Alternatives are: use an "empty" FaceUri, usendn::optional<FaceUri>
, or introduce a new FaceUri scheme such asany://
that has the current semantics (the factory chooses a local endpoint).
Updated by Davide Pesavento over 7 years ago
- Blocked by Feature #4014: ControlCommand: LocalUri in ControlParameters added
Updated by Junxiao Shi over 7 years ago
note-10 questions come from incomplete protocol definition. My recommendations for protocol are:
- For now, protocol should require LocalUri only for Ethernet unicast faces, and forbid LocalUri for UDP and TCP faces.
- In the near future, 409-conflict response should carry LocalUri field.
- In the near future, Uri+LocalUri should be returned on successful face creation response, regardless of whether LocalUri was specified in the request. This would allow
nfdc
to display a good output without retrieving afaces/query
dataset. - In the future, LocalUri can be allowed on UDP and TCP face creation. If so, socket binding should reflect the choice in LocalUri.
- In the future,
faces/create
command can accept multicast face and NIC-associated permanent face creation. This would give more flexibility than through configuration.
Regarding implementation, ndn::optional<FaceUri> localUri
should be passed to ProtocolFactory::createFace
. TcpFactory
and UdpFactory
should throw an exception when localUri
is supplied. EthernetFactory
should throw an exception when localUri
is missing.
Updated by Alex Afanasyev over 7 years ago
I agree with all what Junxiao said, just want to add specifics regarding "protocol forbids". Right now, the command should fail and return to client whatever error code and description. If we use in protocol factories exceptions for this, then exception should be perfect.
Optional for not specified local URI sounds good.
Updated by Eric Newberry over 7 years ago
I agree that LocalUri should be forbidden or UDP and TCP faces and that including the LocalUri with these faces should cause an exception and command failure.
ndn::optional
sounds better than any://
as well.
Updated by Davide Pesavento over 7 years ago
Alex Afanasyev wrote:
If we use in protocol factories exceptions for this, then exception should be perfect.
An exception would do the job, meaning that it would fail the face create command. However the error returned to the client would be something like "internal server error", which is not really correct. On the other hand, triggering the FaceCreationFailedCallback
from ProtocolFactory::createFace
concrete implementations allows specifying a different status code, so that's probably better.
Updated by Eric Newberry over 7 years ago
- Status changed from Code review to Closed