|
Bugzilla – Full Text Bug Listing |
| Summary: | promiscuous code breaks raw packet sockets | ||
|---|---|---|---|
| Product: | ns-3 | Reporter: | Mathieu Lacage <mathieu.lacage> |
| Component: | network | Assignee: | ns-bugs <ns-bugs> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | gjcarneiro |
| Priority: | P1 | ||
| Version: | pre-release | ||
| Hardware: | All | ||
| OS: | All | ||
|
Description
Mathieu Lacage
2008-08-28 18:23:22 UTC
reverting the above-mentioned changeset fixes the problem:
diff -r 92ef80f0352e src/node/packet-socket.cc
--- a/src/node/packet-socket.cc Thu Aug 28 15:06:49 2008 -0700
+++ b/src/node/packet-socket.cc Thu Aug 28 15:39:02 2008 -0700
@@ -353,10 +353,6 @@ PacketSocket::ForwardUp (Ptr<NetDevice>
{
return;
}
- if (packetType != NetDevice::PACKET_HOST)
- {
- return;
- }
PacketSocketAddress address;
looking at this some more, it's pretty clear that the underlying problem here come from the Node::RegisterProtocolHandler API support for promiscuous mode. From the documentation: * \param packetType type of packet received * (broadcast/multicast/unicast/otherhost); Note: * this value is only valid for promiscuous mode * protocol handlers. but, if you look around, you can see that the protocol handlers are never told when they are called in "promiscuous" mode so, the field is invalid but you have no way of knowing whether it is valid or not. Options: 1) add an extra argument to protocol handlers to convey promiscuous state 2) add a value to enum NetDevice::PacketType to document invalidality 3) get rid of both receive callbacks and make the devices themselves report the information so PacketType is valid all the time. 2) is the least intrusive. (In reply to comment #1) > reverting the above-mentioned changeset fixes the problem: > > > diff -r 92ef80f0352e src/node/packet-socket.cc > --- a/src/node/packet-socket.cc Thu Aug 28 15:06:49 2008 -0700 > +++ b/src/node/packet-socket.cc Thu Aug 28 15:39:02 2008 -0700 > @@ -353,10 +353,6 @@ PacketSocket::ForwardUp (Ptr<NetDevice> > { > return; > } > - if (packetType != NetDevice::PACKET_HOST) > - { > - return; > - } > > > PacketSocketAddress address; > This is actually the correct fix. This code is a result of back and forth changes: 1- First you insisted on a single "upcall path" for both promiscuous and non-promiscous packets. I implemented that and had to put the PacketType filtering code in several places to prevent the stack from collapsing as soon as promiscuous mode was activated; 2- Later Craig sent an email saying different paths would be better/safer, but wanted only a single protocol handler callback signature. I changed to do so and non-promiscuous protocol handlers no longer needed the filtering. Thus I removed the filtering code again from several places, but must have forgotten this particular place :-/ Note that there is no "promiscuous mode" as such. There are protocol handlers registered promiscuously (notice the last parameter in Node::RegisterProtocolHandler) which receive a valid PacketType parameter, and there are protocol handlers registered in normally, which receive a dummy PacketType parameter but should not read it. There is no need to pass a parameter to a protocol handler saying if it receives a promiscuous packet or not. The programmer is the one that registers promiscuously or not, and so it is the one that designs the callback to act accordingly to the promiscuous flag. Need to make it so that packet socket regression test fails in this case, then fix it. changeset 01bebf28addd |