Created attachment 142 [details]
Log once TX packet for pcap output

(In reply to comment #1)
> Created an attachment (id=142) [edit]
> Log once TX packet for pcap output


I was expecting a patch which would avoid the echo packet from the csma model instead of ignoring it :)

I am going to change csma-net-device to silently drop packets it receives from the channel that it sent.  Question, should it forwardup broadcast and multicast packets it originates, or is this handled elsewhere?

(In reply to comment #3)
> I am going to change csma-net-device to silently drop packets it receives from
> the channel that it sent.  Question, should it forwardup broadcast and
> multicast packets it originates, or is this handled elsewhere?

Would it not be simpler to modify CsmaChannel::PropagationCompleteEvent to test it->devicePtr against m_currentPacketSender and to make CsmaChannel::TransmitStart take an extra Ptr<NetDevice> argument and store it in m_currentPacketSender ?

That depends on whether or not you ever want to have a device that receives its own transmissions.

Right now every device receives every packet and filters it according to destination, multicast and broadcast address.  One way to proceed is to filter according to source, destination, multicast and broadcast.

As you mention, another way is to filter source in the channel.  However, in reality there is no logic in a wire, so this strikes me as an odd place to perform filtering.  Therefore, I was planning on adding the filtering to the device.

Tom says that he knows of no case where a CSMA device would ever receive a packet that it sends since the receiver is disabled during transmit to avoid spurious collision detection.  Still, I think the right place to do the filtering is on the device and not "inside the wire."

(In reply to comment #5)
> That depends on whether or not you ever want to have a device that receives its
> own transmissions.

I have never seen a device like this.

> Right now every device receives every packet and filters it according to
> destination, multicast and broadcast address.  One way to proceed is to filter
> according to source, destination, multicast and broadcast.
> 
> As you mention, another way is to filter source in the channel.  However, in
> reality there is no logic in a wire, so this strikes me as an odd place to
> perform filtering.  Therefore, I was planning on adding the filtering to the
> device.
> 
> Tom says that he knows of no case where a CSMA device would ever receive a
> packet that it sends since the receiver is disabled during transmit to avoid
> spurious collision detection.  Still, I think the right place to do the
> filtering is on the device and not "inside the wire."

There might be reasons to implement this feature that way because of the current implementation and the output might look exactly the same but this really strikes me as very weird from a modelization perspective. It is also slightly sub-optimal because you have to perform one packet copy per receiver to immediately destroy it in the case the receiver is also the transmitter.

But well, in the end, I cannot make myself care much provided we get nice-looking coherent pcap output.

> this really strikes me as very weird from a modelization perspective.

Hmm.  It strikes me as weird to model a logical filter operation as happening in a copper wire.  It is the device, after all, that turns off its receiver, not the channel.

If I were looking in from the outside, a model of a wire would be about the last place I looked for this filter operation to be happening :-/

> you have to perform one packet copy per receiver
> to immediately destroy it in the case the receiver is also the transmitter.

Well, you have to increment a reference count and then decrement it.

(In reply to comment #7)

> Well, you have to increment a reference count and then decrement it.

no, you have to call Packet::Copy which performs a heap allocation.

>> Well, you have to increment a reference count and then decrement it.

> no, you have to call Packet::Copy which performs a heap allocation.

This is descending into esoterica a little, but okay, you allocate a smart pointer with reference count of 1 to hold the reference to the underlying buffer, the count of which which is incremented and then eventually decremented when the smart pointer reference is decremented and finally the smart pointer is deleted.  Right?  I was trying to make the point that this isn't a copy-the-whole-packet operation, it's logically a reference counting operation.

Anyway, this operation currently happens for all N of the N devices on the channel; putting the source filtering in the channel would allow us to do this (N-1) times at the cost of putting the filtering in a strange (IMO) place.

I lean toward implementing the filtering in the place where it would normally (in the real world) go and not in a place that normally has nothing but copper atoms ;-)

If you can make a case that this has a big performance impact I might be convinced to do what I think is unusual thing in furtherance of speed.  I might also suggest that adding filtering to the device will have negligable performance impact since the copy already happens and I'm just adding a compare.  Perhaps more impact could be made performance-wise by speeding up all N packet operations here and in all other devices with a packet cache to remove the new/delete if we are concerned about the issue.

(In reply to comment #9)
> >> Well, you have to increment a reference count and then decrement it.
> 
> > no, you have to call Packet::Copy which performs a heap allocation.
> 
> This is descending into esoterica a little, but okay, you allocate a smart
> pointer with reference count of 1 to hold the reference to the underlying
> buffer, the count of which which is incremented and then eventually decremented
> when the smart pointer reference is decremented and finally the smart pointer
> is deleted.  Right?  I was trying to make the point that this isn't a
> copy-the-whole-packet operation, it's logically a reference counting operation.

That 'smart pointer' is, however, much bigger than a single count integer. I have not measured it but I expect it to be in the 100 bytes size. I am probably wrong about the exact figure, but, well.

> Anyway, this operation currently happens for all N of the N devices on the
> channel; putting the source filtering in the channel would allow us to do this
> (N-1) times at the cost of putting the filtering in a strange (IMO) place.
> 
> I lean toward implementing the filtering in the place where it would normally
> (in the real world) go and not in a place that normally has nothing but copper
> atoms ;-)
> 
> If you can make a case that this has a big performance impact I might be
> convinced to do what I think is unusual thing in furtherance of speed.  I might
> also suggest that adding filtering to the device will have negligable
> performance impact since the copy already happens and I'm just adding a
> compare.  Perhaps more impact could be made performance-wise by speeding up all
> N packet operations here and in all other devices with a packet cache to remove
> the new/delete if we are concerned about the issue.

I don't think it is a big deal but I did not want to leave uncorrected an incorrect assumption about Packet::Copy :)