Closed brucespang closed 6 years ago
I think we can do this log message! I would like to keep the "+" message on every packet arrival though, since the existing tools depend on that to show the ingress traffic. If a packet gets dropped, we could see a "+" immediately followed by a "d".
What do you think of this as an implementation strategy:
Move the size_bytes() and size_packets() methods from DroppingPacketQueue to AbstractPacketQueue. (The only other child of AbstractPacketQueue is InfinitePacketQueue, and it's very easy to implement these methods there.)
In LinkQueue::read_packet(), keep the call to record_arrival()
and its log entry, and keep the call to (void) packet_queue->enqueue()
, but also check the size_bytes() and size_packets() before and after the enqueue and if they indicate a drop, print a "d" log entry giving the number of packets dropped and the total size of what was dropped.
Keep everything else unchanged.
I think this solves the problem with drop-head that you mentioned, and it also takes away the pressure to correctly implement the bool
return for each different kind of queue. (E.g. I think your CoDel patch might always return true, even when CoDel drops the incoming packet...)
I like your plan, and I've pushed up some code that does that. One potential issue is that if more than one packet is dropped, we don't know the individual sizes for each, so I'm just reporting the average size.
Here's a graph of the queue length over time (maximum queue length is 10) with this patch. One surprising thing was that it goes up to 11, but I think this is because if a packet is not completely sent during a delivery opportunity, it's not marked as delivered until the next time.
Looks pretty good! How would you feel about having the log message just be <timestamp> d <num_dropped> <total_bytes_dropped>
? That way we don't have to worry about the averaging...
I like it, it's a lot cleaner! I've pushed up a change to do that.
It seems like it could be good to document what the different log lines mean, is there a good place to put that?
I'm reasonably happy with the mahimahi(1)
overview manpage, but we never really finished the mm-link(1)
man page...
If you want to go above and beyond, do you want to take a stab at writing the mm-link(1)
manpage from scratch (and include that information)? But feel free to say no as this is not really required to merge this PR. :-)
I don't feel like I understand mm-link well enough to write a manpage for it, but I did add a section to mm-link(1)
on the log format. Feel free to edit it as you see fit!
Merged -- thanks!
Hi!
I've been using pantheon to do some tests of queue lengths for different congestion control algorithms. There was some strange looking behavior, because mahimahi's LinkQueue logs every packet received, not every packet successfully enqueued. Here's a graph of the queue length over time for ten cubic flows with a drop-tail queue with a 100 packet limit, generated from the mahimahi log:
This patch adds a new log entry for dropped packets, which looks like:
[time] d [packet size]
. With the patch, the queue length for ten cubic flows and a maximum queue length of 100 looks more reasonable:One note: with this patch, the drop-head queue won't log dropped packets (it's a little tricky, because we would like to log the size of the dropped packet, not the received packet). If you had a suggestion for how to do that, I'm happy to make the change.