|
Bugzilla – Full Text Bug Listing |
| Summary: | Node::NonPromiscReceiveFromDevice reports a meaningless destination address to user callbacks | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Mathieu Lacage <mathieu.lacage> |
| Component: | network | Assignee: | Craig Dowell <craigdo> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | craigdo, gjcarneiro, ns-bugs, tomh |
| Priority: | P5 | ||
| Version: | pre-release | ||
| Hardware: | All | ||
| OS: | All | ||
The non-defined value is documented: * \param receiver the address of the receiver; Note: this value is * only valid for promiscuous mode protocol * handlers. Changing 'receiver' to the value device->GetAddress() could be incorrect. For instance, broadcast or multicast packets will still be received in non-promiscuous mode, but the destination address will not be the unicast address of the interface. (In reply to comment #1) > The non-defined value is documented: > > * \param receiver the address of the receiver; Note: this value is > * only valid for promiscuous mode protocol > * handlers. > > Changing 'receiver' to the value device->GetAddress() could be incorrect. For > instance, broadcast or multicast packets will still be received in > non-promiscuous mode, but the destination address will not be the unicast > address of the interface. There is no doubt that this value is incorrect in non-promisc mode. I am merely suggesting to make it slightly less incorrect but, hey, I can live without it. Clearly, the idea of using the same callback signature for two different events with different semantics and numbers of arguments was a bad idea. If it was mine, I deserve to be whipped. (In reply to comment #1) > The non-defined value is documented: > > * \param receiver the address of the receiver; Note: this value is > * only valid for promiscuous mode protocol > * handlers. > > Changing 'receiver' to the value device->GetAddress() could be incorrect. For > instance, broadcast or multicast packets will still be received in > non-promiscuous mode, but the destination address will not be the unicast > address of the interface. In WiFi and Csma, this is set to the destination address of the L2 frame. For PointToPoint, we don't really have a L2 destination address conceptually. I would be OK with Mathieu's proposal to change this from "from" to "device->GetAddress()" (seems to be possibly less misleading to an unsuspecting user) and suggest to change the doxygen to: * \param receiver the destination address of the received frame; Note: this value is * only valid for promiscuous mode protocol * handlers. Note2: If the L2 protocol does not use L2 addresses, the address reported here is the value of device->GetAddress(). Added device->GetAddress() in changeset 1f6962c1083c |
In the following function, we can see that we report to the user callback that the destination address of the packet is the same as its source address. bool Node::NonPromiscReceiveFromDevice (Ptr<NetDevice> device, Ptr<const Packet> packet, uint16_t protocol, const Address &from) { NS_LOG_FUNCTION(this); return ReceiveFromDevice (device, packet, protocol, from, from, NetDevice::PacketType (0), false); } A much more helpful value would be something along the lines of: diff -r ba8c1944fb6d src/node/node.cc --- a/src/node/node.cc Tue Jun 22 10:31:51 2010 +0200 +++ b/src/node/node.cc Wed Jun 23 08:21:11 2010 +0200 @@ -260,7 +260,8 @@ const Address &from) { NS_LOG_FUNCTION(this); - return ReceiveFromDevice (device, packet, protocol, from, from, NetDevice::PacketType (0), false); + return ReceiveFromDevice (device, packet, protocol, from, device->GetAddress (), + NetDevice::PacketType (0), false); } bool