Issue 4889046: Added RED queue
|
Created:
13 years, 8 months ago by talau Modified:
13 years, 5 months ago CC:
ns-3-reviews_googlegroups.com Visibility:
Public. |
Patch Set 1 #
Total comments: 41
Patch Set 2 : fixes suggested by Duy and others changes #
Total comments: 38
Patch Set 3 : work in red-queue reviews #
MessagesTotal messages: 14
http://codereview.appspot.com/4889046/diff/1/src/network/examples/red-tests.cc File src/network/examples/red-tests.cc (right): http://codereview.appspot.com/4889046/diff/1/src/network/examples/red-tests.c... src/network/examples/red-tests.cc:25: */ this is great that we provide the plotting tools as well. I think Tom was suggesting whether we can create good known test cases like DropTailQueueTestCase. Perhaps, somewhere along the line with tcl/test/test-suite-red.tcl in ns2. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:96: "True to increases dropping prob. slowly when ave queue exceeds maxthresh", ave? it's better if we spell the description out to make it clear http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:174: RedQueue::SetQueueLimit(uint32_t lim) add a space here SetQueueLimit (uint32_t... http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:182: { SetTh (double... http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:189: RedQueue::GetStats() GetStats () http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:217: */ Why commenting this out? If it's not needed, you should remove it. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:218: NS_LOG_DEBUG("Enqueue in bytes mode"); NS_LOG_DEBUG ("En... could you make sure there it's consistently spaced for every NS_LOG_DEBUG http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:230: */ same thing here http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:245: m = uint32_t(ptc * (now - m_idleTime).GetSeconds()); make sure the coding style is consistently space, GetSeconds () http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:339: */ Does this mean that we need to make RED parameters adaptive to changes in link bandwidth? http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:362: //this comment. use block comments for longer comments http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:444: */ You can remove this block of comments as well http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:462: // pkts: the number of packets arriving in 50 ms use block comments http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:482: // pkts: the number of packets arriving in 50 ms use block comment http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:532: // p = (qAvg - m_minTh) / (maxTh - minTh) if you don't need it, you should remove it http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:586: //p = p * (size / meanPktSize); remove this dead code as well http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:605: else // packets It's better if we make it explicit, else if (GetMode () == PACKETS) else something here
Sign in to reply to this message.
Small comments in Review by Duy; thanks :D http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:96: "True to increases dropping prob. slowly when ave queue exceeds maxthresh", On 2011/08/18 08:01:27, Duy wrote: > ave? it's better if we spell the description out to make it clear Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:174: RedQueue::SetQueueLimit(uint32_t lim) On 2011/08/18 08:01:27, Duy wrote: > add a space here SetQueueLimit (uint32_t... Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:182: { On 2011/08/18 08:01:27, Duy wrote: > SetTh (double... Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:189: RedQueue::GetStats() On 2011/08/18 08:01:27, Duy wrote: > GetStats () Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:217: */ On 2011/08/18 08:01:27, Duy wrote: > Why commenting this out? If it's not needed, you should remove it. Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:218: NS_LOG_DEBUG("Enqueue in bytes mode"); On 2011/08/18 08:01:27, Duy wrote: > NS_LOG_DEBUG ("En... > could you make sure there it's consistently spaced for every NS_LOG_DEBUG Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:230: */ On 2011/08/18 08:01:27, Duy wrote: > same thing here Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:245: m = uint32_t(ptc * (now - m_idleTime).GetSeconds()); On 2011/08/18 08:01:27, Duy wrote: > make sure the coding style is consistently space, > GetSeconds () Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:339: */ On 2011/08/18 08:01:27, Duy wrote: > Does this mean that we need to make RED parameters adaptive to changes in link > bandwidth? If the RED queue is running, yes. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:362: //this comment. On 2011/08/18 08:01:27, Duy wrote: > use block comments for longer comments Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:444: */ On 2011/08/18 08:01:27, Duy wrote: > You can remove this block of comments as well Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:462: // pkts: the number of packets arriving in 50 ms On 2011/08/18 08:01:27, Duy wrote: > use block comments Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:482: // pkts: the number of packets arriving in 50 ms On 2011/08/18 08:01:27, Duy wrote: > use block comment Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:532: // p = (qAvg - m_minTh) / (maxTh - minTh) On 2011/08/18 08:01:27, Duy wrote: > if you don't need it, you should remove it Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:586: //p = p * (size / meanPktSize); On 2011/08/18 08:01:27, Duy wrote: > remove this dead code as well Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:605: else // packets On 2011/08/18 08:01:27, Duy wrote: > It's better if we make it explicit, > > else if (GetMode () == PACKETS) > > else > something here > Done.
Sign in to reply to this message.
Mostly minor spelling in comments. Move documentation of m_qW flags to header? Document m_cautious = 3 in header? http://codereview.appspot.com/4889046/diff/5001/src/network/examples/red-test... File src/network/examples/red-tests.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/examples/red-test... src/network/examples/red-tests.cc:112: // Clientes are in left side "Clients" http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... File src/network/test/red-queue-test-suite.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:173: NS_TEST_EXPECT_MSG_NE (drop.test3, 0, "There should some dropped packets"); "should be some" http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:176: // test 4: reduced maxTh, this cause more drops "causes" http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:192: NS_TEST_EXPECT_MSG_GT (drop.test4, drop.test3, "Test 4 need to have more drops than test 3"); "Test 4 should have..."? also on lines 213, 233, 253 http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:61: * PORT NOTE: Almost all comments also been ported from NS-2 "have also been ported" http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:384: * If m_qW=-2, set it to a reasonable value of 1-exp(-10/C). Since m_qW is accessible by attribute QW, should this documentation of key values be in doxygen (presumably in the header)? http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h File src/network/utils/red-queue.h (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:200: // Min avg length threshold (bytes), should be >= 2*minTh Does the "should" clause belong here? or only on m_maxTh? http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:221: double m_vA; extraneous white space (tabs?) at end of this line and twice more following http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:245: * 1 for not dropping/marking when the instantaneous queue is much below the, red-queue.cc line 235 checks m_cautious = 3, document here?
Sign in to reply to this message.
Some minor comments, other than that I think it's ready. http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... File src/network/test/red-queue-test-suite.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:32: { these test cases are fine. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:215: uint32_t nQueued; initialize nQueued = 0 http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:228: uint32_t m = 0; needs some comments on m like "simulate number of packets arrival during idle period" http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:309: if (m_isNs1Compat) I don't understand why we need NS1 compatibility here. No one is using ns1 anymore, perhaps we can remove it? http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:359: */ This comment is not needed, obviously, if you set the max and min threshold the same, you'll have division by zero. The queue is then not RED anymore but drop-tail with a specified drop threshold. I don't think users would pick something like this http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h File src/network/utils/red-queue.h (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:174: We need comments to describe in detail each of these functions. Especially Estimator, CalculatePNew, ModifyP
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... File src/network/test/red-queue-test-suite.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:32: { I ran this on your test-suite ./test.py -g -v -s red-queue -t error.txt and got valgrind errors, could you take a look again? http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:76: p9 and p10 are not being used
Sign in to reply to this message.
As this implementation is inclined towards the implementation in ns-2 / Floyd's papers, please cite the paper(s) and ns-2 version (if known) in the header. For instance, the role of "gentle", "cautious=1,2,3", "how time constants are chosen" are very ns-2/paper specific details. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:150: { If we intend to use LinkBandwidth and delay , they could be derived from the netdevice that attaches the queue. That is , linkBw and delay can be passed as arguments to the constructor or provide setter functions. If this is not done, the end-user (example file) is always forced to set the BW and delay to set m_ptc correctly, instead of just focussing on QW,Max/Min Th.. Further, the user will be forced to set bw and delay globally for all links.. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:181: RedQueue::SetTh(double min, double max) as min and max are functions/macros in the std namespace it causes portability issues in weak compilers. suggest to use "maxTh", "minTh" http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:202: InitializeParams (); What is the advantage of calling InitializeParams here rather than in the constructor? http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:284: else if (DropEarly(p, nQueued)) DropEarly ( http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:355: double th_diff = (m_maxTh - m_minTh); Have we done tests where max and min are the same, to see if we get DropTail behavior.. This is a very common feature used by students to compare/debug. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (left): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:184: m_minTh = min; Can we NS_ASSERT when max < min?
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (left): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:184: m_minTh = min; On 2011/11/09 00:08:27, John Abraham wrote: > Can we NS_ASSERT when max < min? Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:61: * PORT NOTE: Almost all comments also been ported from NS-2 On 2011/09/02 17:21:35, pdbj wrote: > "have also been ported" Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:215: uint32_t nQueued; On 2011/10/02 00:05:04, Duy wrote: > initialize nQueued = 0 Why? nQueued is every time adjusted by next instructions. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:309: if (m_isNs1Compat) On 2011/10/02 00:05:04, Duy wrote: > I don't understand why we need NS1 compatibility here. No one is using ns1 > anymore, perhaps we can remove it? It is used in some tests in http://icir.org/floyd/papers/redsims.ps So I kept it for validation proposes. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:359: */ On 2011/10/02 00:05:04, Duy wrote: > This comment is not needed, obviously, if you set the max and min threshold the > same, you'll have division by zero. The queue is then not RED anymore but > drop-tail with a specified drop threshold. I don't think users would pick > something like this Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:359: */ On 2011/10/02 00:05:04, Duy wrote: > This comment is not needed, obviously, if you set the max and min threshold the > same, you'll have division by zero. The queue is then not RED anymore but > drop-tail with a specified drop threshold. I don't think users would pick > something like this Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:384: * If m_qW=-2, set it to a reasonable value of 1-exp(-10/C). On 2011/09/02 17:21:35, pdbj wrote: > Since m_qW is accessible by attribute QW, should this documentation of key > values be in doxygen (presumably in the header)? Humm. I think that the comment in attribute QW is enough :)
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h File src/network/utils/red-queue.h (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:200: // Min avg length threshold (bytes), should be >= 2*minTh On 2011/09/02 17:21:35, pdbj wrote: > Does the "should" clause belong here? or only on m_maxTh? Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:221: double m_vA; On 2011/09/02 17:21:35, pdbj wrote: > extraneous white space (tabs?) at end of this line and twice more following Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:245: * 1 for not dropping/marking when the instantaneous queue is much below the, On 2011/09/02 17:21:35, pdbj wrote: > red-queue.cc line 235 checks m_cautious = 3, document here? Done.
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/examples/red-test... File src/network/examples/red-tests.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/examples/red-test... src/network/examples/red-tests.cc:112: // Clientes are in left side On 2011/09/02 17:21:35, pdbj wrote: > "Clients" Done.
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:215: uint32_t nQueued; My compiler is giving me warning about uninitialized variable. On 2011/11/18 13:19:30, talau wrote: > On 2011/10/02 00:05:04, Duy wrote: > > initialize nQueued = 0 > > Why? nQueued is every time adjusted by next instructions.
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... File src/network/test/red-queue-test-suite.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:32: { On 2011/10/02 00:33:26, Duy wrote: > I ran this on your test-suite > ./test.py -g -v -s red-queue -t error.txt > > and got valgrind errors, could you take a look again? > please, what error do you found? is this only in red-queue tests? like: ./test.py -g -v -s drop-tail-queue -t error.txt http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:76: On 2011/10/02 00:33:26, Duy wrote: > p9 and p10 are not being used Done. http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:173: NS_TEST_EXPECT_MSG_NE (drop.test3, 0, "There should some dropped packets"); On 2011/09/02 17:21:35, pdbj wrote: > "should be some" Done. http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:176: // test 4: reduced maxTh, this cause more drops On 2011/09/02 17:21:35, pdbj wrote: > "causes" Done. http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:192: NS_TEST_EXPECT_MSG_GT (drop.test4, drop.test3, "Test 4 need to have more drops than test 3"); On 2011/09/02 17:21:35, pdbj wrote: > "Test 4 should have..."? also on lines 213, 233, 253 Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.c... src/network/utils/red-queue.cc:228: uint32_t m = 0; On 2011/10/02 00:05:04, Duy wrote: > needs some comments on m like "simulate number of packets arrival during idle > period" Done. http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h File src/network/utils/red-queue.h (right): http://codereview.appspot.com/4889046/diff/5001/src/network/utils/red-queue.h... src/network/utils/red-queue.h:174: On 2011/10/02 00:05:04, Duy wrote: > We need comments to describe in detail each of these functions. Especially > Estimator, CalculatePNew, ModifyP Done.
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc File src/network/utils/red-queue.cc (right): http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:181: RedQueue::SetTh(double min, double max) On 2011/11/09 00:08:27, John Abraham wrote: > as min and max are functions/macros in the std namespace it causes portability > issues in weak compilers. suggest to use "maxTh", "minTh" Done. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:202: InitializeParams (); On 2011/11/09 00:08:27, John Abraham wrote: > What is the advantage of calling InitializeParams here rather than in the > constructor? Hum, thanks for the comment. Try it.. some variables aren't stay with the correct value. And this is called only one time. http://codereview.appspot.com/4889046/diff/1/src/network/utils/red-queue.cc#n... src/network/utils/red-queue.cc:284: else if (DropEarly(p, nQueued)) On 2011/11/09 00:08:27, John Abraham wrote: > DropEarly ( Done.
Sign in to reply to this message.
http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... File src/network/test/red-queue-test-suite.cc (right): http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... src/network/test/red-queue-test-suite.cc:32: { I think it's only specific to the red-queue tests? There is memory leaks in it. If you try running it on drop-tail-queue, you won't see any memory leaks. On 2011/11/27 16:02:05, talau wrote: > On 2011/10/02 00:33:26, Duy wrote: > > I ran this on your test-suite > > ./test.py -g -v -s red-queue -t error.txt > > > > and got valgrind errors, could you take a look again? > > > please, what error do you found? is this only in red-queue tests? like: > ./test.py -g -v -s drop-tail-queue -t error.txt
Sign in to reply to this message.
Are these leaks or "still reachable" errors? I tried it out, I found "still reachable" errors, the fix was to add "Simulator::Destroy" after "RunRedTests". On 2011/11/27 17:27:42, Duy wrote: > http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... > File src/network/test/red-queue-test-suite.cc (right): > > http://codereview.appspot.com/4889046/diff/5001/src/network/test/red-queue-te... > src/network/test/red-queue-test-suite.cc:32: { > I think it's only specific to the red-queue tests? There is memory leaks in it. > If you try running it on drop-tail-queue, you won't see any memory leaks. > > On 2011/11/27 16:02:05, talau wrote: > > On 2011/10/02 00:33:26, Duy wrote: > > > I ran this on your test-suite > > > ./test.py -g -v -s red-queue -t error.txt > > > > > > and got valgrind errors, could you take a look again? > > > > > please, what error do you found? is this only in red-queue tests? like: > > ./test.py -g -v -s drop-tail-queue -t error.txt
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
