[Ns-developers] Tracing Rework

[Mathieu Lacage]

On Sun, 2009-03-01 at 13:28 -0800, craigdo at ee.washington.edu wrote:

[snip]

> > 2) In wifi-mac.h: please, don't make the traces protected: if you need
> > to trigger them from subclasses, use a protected Notify method. The same 
> > goes for wifi-phy.h.
> 
> Do you mean that you want the sources wrapped by a function -- as in, for
> example, NotifyRxDropTrace ()?  That seems like A Good Thing (TM).

yes, indeed.

> > 3) WifiNetDevice::SniffPacket ?? You are making an extra copy of the
> packet
> > just for tracing, even if there are no connected trace sinks ? This really
> does
> > not look like a good idea because it is emulating something really
> stupid/bad
> > from linux (which, incidentely, I believe got fixed in recent wifi stacks)
> and
> > because its performance cost is a bit horrid.
> 
> This actually happens all over our code.  It is so that various trace
> sources return some relatively sane number of variations on packet contents.
> Even in the simplest cases, CSMA, for example could trace a complete packet,
> a packet without an ethernet trailer, a packet without an ethernet header, a
> packet with or without an LLC/SNAP header, etc., depending on when a trace
> is hit.  Making a copy of the original packet that came in over the channel
> allows for only one main variation -- DIX or LLC/SNAP.  If this is horrid
> and should be optimized, it should be done, well, everywhere.

I don't mind the packet copy itself: I mind the following addition of an
ethernet header because:

1) it's horrid to make a wifi device look like an ethernet device. A
real bad linux idea, and, one I believe decent and recent linux drivers
don't do anymore (I could be wrong here though).

2) it's horribly inefficient to add the ethernet header because it is
the addition of the ethernet header which will trigger a deep copy. The
previous call to Copy itself is really cheap so, it's not a big deal.

What I would support is a Copy of the packet before the wifi header is
removed because that would be both efficient and result in traces which
do not contain an ethernet header but contain a wifi header (but, in
this case, you would not need a copy anymore). It might be possible to
partially implement this by replacing RemoveHeader by PeekHeader in
MacLow::ReceiveOk and calling a trace hook in the right location in this
function (to filter out some unwanted packets). That would not be all
the filtering done by a real pcap trace on a real linux system, but,
well...

> The other version, the non-promiscuous sniffer is a bit more problematic.
> Currently, pcap traces written by most of our devices look this way and so I
> think current users will expect to be able to see it.  It can be convenient
> to see a filtered subset of the frames flowing through the device.  This is
> mostly easy to do in simple net devices where everything is available in one
> place.  Not so much for wifi where complexity issues keep things
> modularized, and modularity is the enemy of this particular trace source.
> This is especially visible in the wifi code.  It seems very complicated, way
> more trouble than its worth, to make this work ... correctly.  It breaks all
> kinds of modularity and spans layers.  Really ugly.

Agreed.

> I can then 
> 
> 1) Remove the objectionable Sniffer (Ethernet frame) and tx complete trace
> sources from the wifi device;
> 
> 2) Remove the unused rx start trace sources from the csma and point-to-point
> devices;
> 
> 3) Remove the non-promiscuous sniffer helper code from the wifi-helper and
> leave yans-wifi-phy hooked to the promisc sniffer;
> 
> 4) Write some words for the manual on this stuff.

I think that the above would be fine.

> 4) Optimizing the packet trace hooks to not copy packets whenever possible.

As I said before, calling Packet::Copy is not a big deal.

Mathieu