Bug #1413
closed
nfdc can create face to same peer more than once
100%
Description
I assume this is not the expected behavior, but please let me know if I'm missing anything here:
Summary:
- nfdc create tcp4 face ==> new face #x.
- nfdc create the same face ==> nfd-status presents multiple lines for the same uri, both attached to the same face #y != #x
- nfdc destroy #y ==> nfd-status still presents the uri, but attach it to an invalid face #.
Sample run:
$ build/bin/nfdc create tcp4://192.168.1.2
Face creation succeeded: ControlParameters(FaceId: 8, Uri: tcp4://192.168.1.2, )
$ build/bin/nfd-status
(partial output):
faceid=8 uri=tcp4://192.168.1.2:6363 counters={in={0i 0d} out={0i 0d}}
$ build/bin/nfdc create tcp4://192.168.1.2
Face creation succeeded: ControlParameters(FaceId: 11, Uri: tcp4://192.168.1.2, )
$ build/bin/nfd-status
(partial output):
faceid=11 uri=tcp4://192.168.1.2:6363 counters={in={0i 0d} out={0i 0d}}
faceid=11 uri=tcp4://192.168.1.2:6363 counters={in={0i 0d} out={0i 0d}}
$ build/bin/nfdc destroy 11
Face destroy succeeded: ControlParameters(FaceId: 11, )
$ build/bin/nfd-status (partial output):
faceid=18446744073709551615 uri=tcp4://192.168.1.2:6363 counters={in={0i 0d} out={0i 0d}}
Updated by Alex Afanasyev over 10 years ago
- Category set to Forwarding
- Assignee set to Alex Afanasyev
- Target version set to v0.1
As I see it, we mistakenly add the same face twice (or any number of times) into FaceTable. In particular, FaceTable::add doesn't check if face has already been added (m_faces.count(face->getId()) > 0). I will submit a patch that addresses the problem.
Updated by Alex Afanasyev over 10 years ago
- Status changed from New to Code review
- % Done changed from 0 to 100
Updated by Junxiao Shi over 10 years ago
- Subject changed from nfdc / nfd-status: create of the face more than once to nfdc can create face to same peer more than once
FaceTable::add can check:
if (face->getId() != INVALID_FACEID)
return;
Similarly FaceTable::remove should return if faceId is INVALID.
Or, should the caller (FaceManager) do this check? (and FaceTable can ASSERT)?
Updated by Alex Afanasyev over 10 years ago
It's easier and more error-prone to do in FaceTable. There are several places where add is called and adding checks in all places would be too much.
In my submitted patch I actually do both check... just in case.
Updated by Alex Afanasyev over 10 years ago
- Status changed from Code review to Closed