Task #1213
closedIncomingFaceId in LocalControlHeader
100%
Description
When a Interest or Data packet is forwarded to a Face that has requested incoming FaceId, attach a LocalControlHeader on the packet, and set incoming FaceId field.
Files
Updated by Alex Afanasyev almost 11 years ago
There is a small problem. At the moment packet is transmitted to the face, there is no longer information about where the packet came from... Therefore, we have to somehow start tracking the face for each received packet... Another option, we have to make decision about the local control header somewhere in the Forwarder.
Updated by Junxiao Shi almost 11 years ago
This task has Category=Forwarding.
Attaching incoming FaceId is part of forwarding, not part of face.
Updated by Junxiao Shi almost 11 years ago
- Due date deleted (
01/31/2014) - Start date deleted (
01/31/2014)
Updated by Junxiao Shi almost 11 years ago
- Assignee set to Junxiao Shi
- Priority changed from Normal to Urgent
I'm raising priority because NRD mock implementation needs this feature. I'm waiting for #1170 to complete.
Updated by Junxiao Shi almost 11 years ago
- Subject changed from Incoming FaceId in LocalControlHeader to IncomingFaceId in LocalControlHeader
- Category changed from Forwarding to Faces
- Status changed from New to In Progress
- Start date set to 02/18/2014
Category=Faces because forwarding should always setIncomingFaceId
.
Face could make a decision on whether to enclose Interest/Data in LocalControlHeader
based on isLocalControlHeaderEnabled
.
Updated by Junxiao Shi almost 11 years ago
- % Done changed from 0 to 20
Blocked on API problem.
ndn-cpp-dev commit d31f4bfb225f7b9a65969423378b8a83afa15f2f:
ndn::Interest
and ndn::Data
store the argument into LocalControlHeader.
The same Interest/Data object is used for outgoing, and the app may not want to receive LocalControlHeader.
- (a) Clearing LocalControlHeader would lose
getIncomingFaceId
information. - (b) Cloning the Interest/Data for outgoing is not efficient.
Updated by Alex Afanasyev almost 11 years ago
For now (to not block testing), I would suggest ignoring this problem and re-use the same LocalControlHeader for all faces that enabled localcontrolheder.
I'm working on a solution for this. The idea is to use non-consecutive memory block within the same send operation (this is actually supported by boost::asio).
Updated by Alex Afanasyev almost 11 years ago
Btw. The problem you mentioned does not really exist. When face has not control header enabled, the current API provides a way to send just wire format of Interest/Data. Basically, you should use condition and either directly use interest.wireEncode()
or interest.getLocalControlHeader().wireEncode(interest)
.
Updated by Junxiao Shi almost 11 years ago
Consider the following:
- Interest arrives with nexthopFaceId field.
- Interest is forwarded to a face which needs incomingFaceId.
It's wrong to have nexthopFaceId field in an outgoing packet.
For now, I'll copy every packet.
Updated by Alex Afanasyev almost 11 years ago
- File LocalControlHeader.svg LocalControlHeader.svg added
What about different approach (attached image), completely separating LocalControlHeader from Face abstraction (since it only applies to local faces and we shouldn't have any overhead for all other types of faces).
Updated by Junxiao Shi almost 11 years ago
I disagree with this proposal due to the following reasons:
- The performance gain is negligible compared to 359,1 when LocalControlHeader is not enabled on a face. The saving is one conditional statement.
- There is no benchmark showing the current design has a bottleneck due to the addition of LocalControlHeader.
- The proposal complicates the implementation of every Face type that may be used for local. Instead of one TcpFace class, it requires a TcpFace class for remote communication, and a TcpLocalFace class for local communicate. This contradicts with simplicity design goal.
Updated by Alex Afanasyev almost 11 years ago
I would argue about simplicity. Forcing every face to deal (implicitly or explicitly) with local control header is wrong. TcpFace that is used for local communications should be different from face that is used for non-local. The clearer this distinction, less errors (I mean security errors) people will be able to make.
http://gerrit.named-data.net/#/c/359/1 is no less confusing, btw. And it is also kind of wrong, since Face's sendInterest/Data calls will never be called (they are virtual). Making them non-virtual eliminates an opportunity for custom face implementation to deal with Interest/Data directly as they wish...
Updated by Junxiao Shi almost 11 years ago
Whether a Face is local or not is a policy decision. It has nothing to do with the underlying protocol. You are using the same TCP for local and non-local communications, so there is no fundamental difference.
Face::sendInterest
and Face::sendData
are virtual. However, not every Face implementation needs to override them.
A Face implementation can choose to:
- override
Face::sendLinkPacket
, to send the packet with LocalControlHeader attached if enabled -- UNIX and TCP must use this - or, override
Face::sendInterest
andFace::sendData
, so that it can process Interest and Data differently, but cannot attach LocalControlHeader -- multicast must use this
Updated by Junxiao Shi almost 11 years ago
- Assignee changed from Junxiao Shi to Alex Afanasyev
- % Done changed from 60 to 20
20140219 conference call decides to go with Alex's design, and the task is reassigned.
Updated by Alex Afanasyev almost 11 years ago
- Status changed from In Progress to Code review
- % Done changed from 80 to 100
Updated by Alex Afanasyev almost 11 years ago
- Status changed from Code review to Closed