Issue 338100043: Mininet Test Automation
|
Created:
6 years, 5 months ago by Michail.Xirouchakis Modified:
6 years, 5 months ago Reviewers:
engr.sikandar.iqbal, 2016shana2333, Vasileios Papageorgiou, dkostic, younes.tahri3, paul.busi38, alireza.farshin CC:
sdnik2200ht17_ict.kth.se Visibility:
Public. |
DescriptionMininet test file
Run from Mininet CLI (after you have launched the topology) by typing
source test-topo-1-csd
Demo youtube video:
https://www.youtube.com/watch?v=fqUU4qAxApY
BUG=
Patch Set 1 #
Total comments: 25
MessagesTotal messages: 6
Please review my code regarding test automation with Mininet. It's just this file, nothing else needed. Runs from Mininet CLI as described.
Sign in to reply to this message.
Hi Michael, Check my comments for mininet test automation file. BR, Vasileios Papageorgiou https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-csd File mininet_testing/test-topo-1-csd (right): https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-c... mininet_testing/test-topo-1-csd:1: py "[TEST-1] Pingall to discover all hosts..." I noticed from the youtube tutorial video that we still need to write manually switches so that hosts are connecting into them. But this has to do with the topology standford implementation.In that case we need to define on this file the exact number of switches that generate traffic. Since, we have not figured out how to make it more abstract so far it is good way of implementing iperf generation as it is written on the file. https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-c... mininet_testing/test-topo-1-csd:9: py "[TEST-1] Sleeping for 60 seconds." we can reduce the sleeping time to 30 seconds upon discussion we had on Friday so that we can reduce the completion time of the whole test.
Sign in to reply to this message.
Hi Michael, I made some comments on CPM latest update. BR, Vasileios Papageorgiou https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py File cpm/cpm_13.py (right): https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode47 cpm/cpm_13.py:47: reset_topo_link_json_obj = {'src_switch': '', Is this json object used for the graphs that DM is going to print? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode61 cpm/cpm_13.py:61: params_dict = {'module': 'nfm', 'row': 1, 'table': 'port_counters'} if you want to update flow counters do you need to define another params_dict like this : params_dict1 = {'module': 'nfm', 'row': 2, 'table': 'flow_counters'} ? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode106 cpm/cpm_13.py:106: For defining the priorities on lines 109, 114, 119, 122 do we consider admission control logic? Are these priorities enough to distinguish ICMP , TCP and UDP? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode186 cpm/cpm_13.py:186: def _deny_flow(self, datapath, in_port, pkt_in, hard_timeout=1): Has this been tested in terms of denying a flow based on priorities? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode243 cpm/cpm_13.py:243: # Inform DM about this link So now based on the reset you did in line 47 you are initializing the json object that the dm is going to use for plotting the graphs? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode293 cpm/cpm_13.py:293: # Weights for composite metric: When the RPM is ready we need to add the RPM metric right? https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode478 cpm/cpm_13.py:478: # Block all similar future packets for 1 second: Commented lines 478 -488 are for future implementation when we will need to deny flows and etc.?
Sign in to reply to this message.
@Vasileios I have replied to your comments regarding the current CPM implementation. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py File cpm/cpm_13.py (right): https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode47 cpm/cpm_13.py:47: reset_topo_link_json_obj = {'src_switch': '', On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > Is this json object used for the graphs that DM is going to print? Yes. Paul (DM) wanted an one-time-message from CPM, so that DM knows when a new topology is being launched. The DM will react to this message by cleaning up (dropping) its stored database data. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode61 cpm/cpm_13.py:61: params_dict = {'module': 'nfm', 'row': 1, 'table': 'port_counters'} On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > if you want to update flow counters do you need to define another params_dict > like this : params_dict1 = {'module': 'nfm', 'row': 2, 'table': 'flow_counters'} > ? Yes. We currently don't use Flow Counters at all and I don't know why/when we will do this. Maybe never. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode106 cpm/cpm_13.py:106: On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > For defining the priorities on lines 109, 114, 119, 122 do we consider admission > control logic? Are these priorities enough to distinguish ICMP , TCP and UDP? No. Admission logic looks at packet headers (e.g. IP Proto or TP port numbers) to classify traffic. This has nothing to do with OF priorities. The values I put are just arbitrary as we never have two rules matching the same packet; maybe we will have in the future. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode186 cpm/cpm_13.py:186: def _deny_flow(self, datapath, in_port, pkt_in, hard_timeout=1): On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > Has this been tested in terms of denying a flow based on priorities? Yes; the function has been tested and it denies traffic for 'hard_timeout' number of seconds. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode243 cpm/cpm_13.py:243: # Inform DM about this link On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > So now based on the reset you did in line 47 you are initializing the json > object that the dm is going to use for plotting the graphs? CPM is telling the DM which links exist in the topology. Each link is reported twice; this is the nature of the Ryu Topology API. For example, link between switch 303 and 204 is reported as src_switch 303, dst_switch 204 and once more with src_switch 204, dst_switch 303. This is not a problem though, just a detail. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode293 cpm/cpm_13.py:293: # Weights for composite metric: On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > When the RPM is ready we need to add the RPM metric right? That is correct. I personally don't think that is gonna happen (RPM work well) so I have not focused in making this code better. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode478 cpm/cpm_13.py:478: # Block all similar future packets for 1 second: On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > Commented lines 478 -488 are for future implementation when we will need to deny > flows and etc.? Well, deny_flow() can be used to block a type of traffic. However there is currently no logic as to 'when' or 'why' to deny traffic. This needs to be implemented. The rest of the commented lines can be deleted.
Sign in to reply to this message.
@Vasileios Thanks for the comments on the mininet CLI scripting file (test-topo-1-csd). When we reduce the number of hosts to 4, as dictated by the exercise, we can create a new file. https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-csd File mininet_testing/test-topo-1-csd (right): https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-c... mininet_testing/test-topo-1-csd:1: py "[TEST-1] Pingall to discover all hosts..." On 2017/11/25 18:58:03, Vasileios Papageorgiou wrote: > I noticed from the youtube tutorial video that we still need to write manually > switches so that hosts are connecting into them. But this has to do with the > topology standford implementation.In that case we need to define on this file > the exact number of switches that generate traffic. Since, we have not figured > out how to make it more abstract so far it is good way of implementing iperf > generation as it is written on the file. Acknowledged. https://codereview.appspot.com/338100043/diff/1/mininet_testing/test-topo-1-c... mininet_testing/test-topo-1-csd:9: py "[TEST-1] Sleeping for 60 seconds." On 2017/11/25 18:58:03, Vasileios Papageorgiou wrote: > we can reduce the sleeping time to 30 seconds upon discussion we had on Friday > so that we can reduce the completion time of the whole test. Acknowledged.
Sign in to reply to this message.
Hi Michael, Thanks for replying to my comments regarding your code implementation. BR, Vasileios Papageorgiou https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py File cpm/cpm_13.py (right): https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode47 cpm/cpm_13.py:47: reset_topo_link_json_obj = {'src_switch': '', On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > Is this json object used for the graphs that DM is going to print? > > Yes. Paul (DM) wanted an one-time-message from CPM, so that DM knows when a new > topology is being launched. The DM will react to this message by cleaning up > (dropping) its stored database data. Acknowledged. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode61 cpm/cpm_13.py:61: params_dict = {'module': 'nfm', 'row': 1, 'table': 'port_counters'} On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > if you want to update flow counters do you need to define another params_dict > > like this : params_dict1 = {'module': 'nfm', 'row': 2, 'table': > 'flow_counters'} > > ? > > Yes. We currently don't use Flow Counters at all and I don't know why/when we > will do this. Maybe never. ok! maybe is not nesessary https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode106 cpm/cpm_13.py:106: On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > For defining the priorities on lines 109, 114, 119, 122 do we consider > admission > > control logic? Are these priorities enough to distinguish ICMP , TCP and UDP? > > No. Admission logic looks at packet headers (e.g. IP Proto or TP port numbers) > to classify traffic. > This has nothing to do with OF priorities. The values I put are just arbitrary > as we never have two rules matching the same packet; maybe we will have in the > future. Acknowledged. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode186 cpm/cpm_13.py:186: def _deny_flow(self, datapath, in_port, pkt_in, hard_timeout=1): On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > Has this been tested in terms of denying a flow based on priorities? > > Yes; the function has been tested and it denies traffic for 'hard_timeout' > number of seconds. Done. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode243 cpm/cpm_13.py:243: # Inform DM about this link On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > So now based on the reset you did in line 47 you are initializing the json > > object that the dm is going to use for plotting the graphs? > > CPM is telling the DM which links exist in the topology. Each link is reported > twice; this is the nature of the Ryu Topology API. For example, link between > switch 303 and 204 is reported as src_switch 303, dst_switch 204 and once more > with src_switch 204, dst_switch 303. This is not a problem though, just a > detail. Acknowledged. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode293 cpm/cpm_13.py:293: # Weights for composite metric: On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > When the RPM is ready we need to add the RPM metric right? > > That is correct. I personally don't think that is gonna happen (RPM work well) > so I have not focused in making this code better. Let's hope that we have soon results. https://codereview.appspot.com/338100043/diff/1/cpm/cpm_13.py#newcode478 cpm/cpm_13.py:478: # Block all similar future packets for 1 second: On 2017/11/27 11:29:24, Michail.Xirouchakis wrote: > On 2017/11/26 10:38:04, Vasileios Papageorgiou wrote: > > Commented lines 478 -488 are for future implementation when we will need to > deny > > flows and etc.? > > Well, deny_flow() can be used to block a type of traffic. However there is > currently no logic as to 'when' or 'why' to deny traffic. This needs to be > implemented. The rest of the commented lines can be deleted. Ok! As we discussed today i am going to work on that implementation.
Sign in to reply to this message.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||
