Network Tester Patch
Review Request #561 - Created March 11, 2011 and submitted
| Information | |
|---|---|
| Tushar Krishna | |
| gem5 | |
| Reviewers | |
| Default | |
| ali, beckmann, gblack, nate, stever | |
This patch adds the network tester for simple and garnet networks. The tester code is in testers/networktest. The tester can be invoked by configs/example/ruby_network_test.py. A dummy coherence protocol called Network_test is also addded for network-only simulations and testing. The protocol takes in messages from the tester and just pushes them into the network in the appropriate vnet, without storing any state.
Testing done with garnet and simple networks. I will update config assumptions etc on the wiki.
Review request changed
Updated (March 11, 2011, 1:33 a.m.)
People: |
|
|---|
Posted (March 11, 2011, 3:39 a.m.)
Hi Tushar, I have a few minor comments below, but my major question is why does the config file create two versions of physical memory? Do they really need to be accessed? I suspect when we create this patch 1.5 years ago, that was required. However since then, we added a feature that prevents the rubyport from accessing physical memory when it is unecessary. I think this patch should use that feature. Also once we add this patch to the repo, we should add the network test + garnet to the regression tester. I'll add that to my list of outstanding regression tests.
-
configs/example/ruby_network_test.py (Diff revision 1) -
The network test doesn't actually do any functional checks, correct? If so, it seems that we can remove this line and instead tell the rubyport not to access physical memory. For instance: "ruby_port.access_phys_mem = False"
-
src/cpu/testers/networktest/networktest.cc (Diff revision 1) -
Expand this comment a bit. This is not straightforward.
-
src/cpu/testers/networktest/networktest.cc (Diff revision 1) -
Please remove commented line. Also if the network tester doesn't do any functional operation, why is funcmem specified in the config file?
-
src/mem/protocol/Network_test-cache.sm (Diff revision 1) -
Please remove comment
-
src/mem/protocol/Network_test-cache.sm (Diff revision 1) -
Please remove commented line
-
src/mem/protocol/Network_test-cache.sm (Diff revision 1) -
You should add a comment here because it is not obvious what you're doing.
-
src/mem/protocol/Network_test-dir.sm (Diff revision 1) -
This comment should be removed.
-
src/mem/protocol/Network_test-dir.sm (Diff revision 1) -
Please remove the commented out lines.
Review request changed
Updated (March 14, 2011, 6:14 p.m.)
Change Summary:
addressed Brad's comments. Specifically: removed second physical memory from ruby_network_tester.py, and added comments in networktest.cc and Network_test-cache/dir.sm files
Posted (March 15, 2011, 4:39 a.m.)
Hi Tushar, This patch basically looks done to me. I just have a few minor comments below. It seems that there are a few parameters that aren't used and should be removed. Also I think you should add a better explanation of the expected values for the injection rate.
-
configs/example/ruby_network_test.py (Diff revision 2) -
Why multiply by 1000? What is the expected range of injection rate?
-
src/cpu/testers/networktest/NetworkTest.py (Diff revision 2) -
Is this parameter used? If so, what does address to trace mean?
-
src/cpu/testers/networktest/NetworkTest.py (Diff revision 2) -
I think you can remove the functional port now, correct?
-
src/cpu/testers/networktest/networktest.cc (Diff revision 2) -
Again, I think you can remove the functional port.
Review request changed
Updated (March 19, 2011, 10:11 p.m.)
Change Summary:
- Addressed Brad's comments: (1) removed functional ports from testers/networktest; (2) added a precision option to ruby_network_test.py that specifies the number of digits of precision after the decimal point for the injection rate. Also added comment to explain injection rate range. This field is used by networktest.cc when deciding whether or not to generate a packet. [Earlier this precision was hard-coded to 3 digits] - Updated the Network_test-cache.sm file to replace CacheRequestType to RubyRequestType in accordance with recent m5 changes.
Diff: |
Revision 3 (+1294 -24)
|
|---|
Review request changed
Updated (March 19, 2011, 10:23 p.m.)
Change Summary:
clearer explanation about injectionrate value in ruby_network_test.py
Diff: |
Revision 4 (+1294 -24)
|
|---|