Issue 329400043: RPM comupting the RTT of each switch
|
Created:
6 years, 6 months ago by younes.tahri3 Modified:
6 years, 4 months ago Reviewers:
engr.sikandar.iqbal, 2016shana2333, Vasileios Papageorgiou, dkostic, paul.busi38, michail.xirouchakis, alireza.farshin CC:
sdnik2200ht17_ict.kth.se Visibility:
Public. |
DescriptionRPM comupting the RTT of each switch
BUG=
Patch Set 1 #
Total comments: 3
Patch Set 2 : RPM sending to DB #
Total comments: 12
Patch Set 3 : update RPM #
Total comments: 10
Patch Set 4 : traffic gen and test automation for RPM #Patch Set 5 : graph generation #
Total comments: 14
Patch Set 6 : Commented RPM code #Patch Set 7 : admission control test file #Patch Set 8 : Admission control test file #Patch Set 9 : update #
Total comments: 3
MessagesTotal messages: 29
Hey! If you have some time you can review my code for RPM. This code computes the RPM for each switch, but only one time.
Sign in to reply to this message.
Hey there! Here is an updated version of the RPM! Feel free to review when you have time! PS: there is no difference betwwen the old version and the new version in Codereview, but that's because I forgot to do a git-cl upload before to do the push
Sign in to reply to this message.
To view the full code, just click on context : whole file <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign...> Virus-free. www.avast.com <https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign...> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2> On Thu, Oct 12, 2017 at 10:08 PM, <younes.tahri3@gmail.com> wrote: > Hey there! > Here is an updated version of the RPM! > Feel free to review when you have time! > > PS: there is no difference betwwen the old version and the new version > in Codereview, but that's because I forgot to do a git-cl upload before > to do the push > > https://codereview.appspot.com/329400043/ >
Sign in to reply to this message.
Hi Yonues! I made a review on your first version of RPM https://codereview.appspot.com/329400043/diff/1/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/1/RPM/RPM.py#newcode11 RPM/RPM.py:11: class L2Switch(app_manager.RyuApp): Probably change the name of the class to RPM https://codereview.appspot.com/329400043/diff/1/RPM/RPM.py#newcode15 RPM/RPM.py:15: super(L2Switch, self).__init__(*args, **kwargs) here as well change it to RPM https://codereview.appspot.com/329400043/diff/1/RPM/RPM.py#newcode36 RPM/RPM.py:36: #for i in self.switches: do we need for loop?
Sign in to reply to this message.
Hi Younes! I made some comments on your new code for RPM https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode50 RPM/RPM.py:50: def roulette(self,ratio): What is the use of this method? Is it used to create random number for swithces? https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode93 RPM/RPM.py:93: self.thread[switch_id].start() are you achieving thread synchronization in here?
Sign in to reply to this message.
I'm looking forward to knowing your opinion regarding my comments. Thanks BR, Sikandar Iqbal https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode33 RPM/RPM.py:33: self.only_once[new_switch.id]=1 what is representation 1 for? https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode54 RPM/RPM.py:54: return 1 what 1 and 0 represent? https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode57 RPM/RPM.py:57: def threaded_function (self, datapath, switch_id): Sorry I couldn't understand that you defined datapath and switch_id are equal and in barrier request method and here you are passing both datapath and switch_id as arguments?
Sign in to reply to this message.
https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode50 RPM/RPM.py:50: def roulette(self,ratio): On 2017/10/12 20:27:00, Vasileios Papageorgiou wrote: > What is the use of this method? Is it used to create random number for swithces? it is used to pick the switches that we would want the RTT from https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode93 RPM/RPM.py:93: self.thread[switch_id].start() On 2017/10/12 20:27:00, Vasileios Papageorgiou wrote: > are you achieving thread synchronization in here? No, I'm creating a thread for each switch
Sign in to reply to this message.
Hey there, I forgot to publish my comments. Here are my replies. I hope it's more clear now https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode33 RPM/RPM.py:33: self.only_once[new_switch.id]=1 On 2017/10/16 22:02:25, engr.sikandar.iqbal wrote: > what is representation 1 for? It's just a way of calling the function using (if only once==1) only one time https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode54 RPM/RPM.py:54: return 1 On 2017/10/16 22:02:25, engr.sikandar.iqbal wrote: > what 1 and 0 represent? 1: We get the switch's RTT 0:We don't(sleep) https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode57 RPM/RPM.py:57: def threaded_function (self, datapath, switch_id): On 2017/10/16 22:02:25, engr.sikandar.iqbal wrote: > Sorry I couldn't understand that you defined datapath and switch_id are equal > and in barrier request method and here you are passing both datapath and > switch_id as arguments? datapath and switch_id are not equal. The datapath is how the controller connects to the switch, and the switch_id is just the switch id. They are arguments here because this function is called for each switch
Sign in to reply to this message.
Thanks! https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode33 RPM/RPM.py:33: self.only_once[new_switch.id]=1 On 2017/10/17 08:38:22, younes.tahri3 wrote: > On 2017/10/16 22:02:25, engr.sikandar.iqbal wrote: > > what is representation 1 for? > > It's just a way of calling the function using (if only once==1) only one time Thanks! https://codereview.appspot.com/329400043/diff/20001/RPM/RPM.py#newcode54 RPM/RPM.py:54: return 1 On 2017/10/17 08:38:22, younes.tahri3 wrote: > On 2017/10/16 22:02:25, engr.sikandar.iqbal wrote: > > what 1 and 0 represent? > > 1: We get the switch's RTT > 0:We don't(sleep) ok, i got it
Sign in to reply to this message.
Here is an updated code for the RPM, feel free to give comments. Testing the fact that a switch that receives commands from the controller has a higher RPM. +Starting to test how traffic affects the RPM
Sign in to reply to this message.
Hi Younes, See the comments for RPM implementation. BR, Vasilis Papageorgiou https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode45 RPM/RPM.py:45: self.iterator_302=0 why do we need self.iterator_302 for switch 302? Can this be done by using a generic self.iterator ? https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode128 RPM/RPM.py:128: def threaded(self): # 1 thread for all the switches Is the method threaded lines 128-147 used? I supposed it has to be used. IF it is still used i cannot see where it is called on the program. I believe that we should have self.threaded() somewhere. Please correct me if i am wrong https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode160 RPM/RPM.py:160: if switch_id==302: i will check the switch id of another one as well to compare the RTT times of multiple switches that have rules. For instance, it would be necessary to install flows on switch 301 and compare the RTT of 301 with the RTT of 302 https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode198 RPM/RPM.py:198: lines 198-205 should be overwritten and delete both of them. This is based on the MyHTTPClient() class that is made for supporting GET, POST requests. https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode207 RPM/RPM.py:207: # self.thread =Thread(target = self.threaded,args=()) What about using ryu thread function ? For instance, self.monitor_thread = hub.spawn(self._monitor) was used on nfm to deal with Thread functionality
Sign in to reply to this message.
https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode45 RPM/RPM.py:45: self.iterator_302=0 On 2017/11/14 13:00:01, Vasileios Papageorgiou wrote: > why do we need self.iterator_302 for switch 302? > Can this be done by using a generic self.iterator ? This is just a tool to test how the traffic affects the RTT of s302. This is just a simple iterator that, when equal to 50s or 100s, gets the avg RTT for this switch https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode128 RPM/RPM.py:128: def threaded(self): # 1 thread for all the switches On 2017/11/14 13:00:01, Vasileios Papageorgiou wrote: > Is the method threaded lines 128-147 used? I supposed it has to be used. IF it > is still used i cannot see where it is called on the program. I believe that we > should have self.threaded() somewhere. Please correct me if i am wrong this was used because the TA told us to try it, remember? 1 thread for all the switchs but then the handler was stuck in the loop and could compute the right RTT(check email) So no, this is not used anymore, the function used is threaded_function https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode160 RPM/RPM.py:160: if switch_id==302: On 2017/11/14 13:00:01, Vasileios Papageorgiou wrote: > i will check the switch id of another one as well to compare the RTT times of > multiple switches that have rules. For instance, it would be necessary to > install flows on switch 301 and compare the RTT of 301 with the RTT of 302 Yes, this test is interesting, but to do so, maybe we need to separete the monitoring function from the rule sending function, so another file( check my last email) I already tried this methode before without much success, but I'll try again, some stuff have been changed since. https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode198 RPM/RPM.py:198: On 2017/11/14 13:00:01, Vasileios Papageorgiou wrote: > lines 198-205 should be overwritten and delete both of them. This is based on > the MyHTTPClient() class that is made for supporting GET, POST requests. I don't understand what you mean here, can you clarify? https://codereview.appspot.com/329400043/diff/40001/RPM/RPM.py#newcode207 RPM/RPM.py:207: # self.thread =Thread(target = self.threaded,args=()) On 2017/11/14 13:00:01, Vasileios Papageorgiou wrote: > What about using ryu thread function ? > For instance, self.monitor_thread = hub.spawn(self._monitor) > was used on nfm to deal with Thread functionality oh I didn't know about this, maybe this could be the solution
Sign in to reply to this message.
Traffic generation : one terminal for each host+ssh+iperf +Test automation for RPM: only one script to execute : autorun.sh
Sign in to reply to this message.
Here you'll find the code I use to generate the graphs that are on the weekly report
Sign in to reply to this message.
Hi Younes, Check my comments for RPM.py new file. BR, Vasileios Papageorgiou https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode1 RPM/RPM.py:1: from ryu.base import app_manager How you run the file to see the graphs? I checked by running ryu-manager RPM.py simple_switch_13.py and on another terminal for topology : sudo mn --custom topo-stanford-campus.py --topo mytopo,s303,s304,s313,s314 --controller remote --mac However, i see empty graphs been created. https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode25 RPM/RPM.py:25: def __init__(self, *args, **kwargs): too many default parameters at _init_() . Delete the unnecessary ones. You can use dictionaries to avoid have extended default values , as well. https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode113 RPM/RPM.py:113: def addRTT302_flows(self): #Computes avg RTT for s302 and s314 each time we add flows(here, 1 iperf flow added each 50s) add more comments here to make the code understandable https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode130 RPM/RPM.py:130: def plot_RTTrules(self): #plot graph for s302 and s314 : RTT function of nb of rules sent to s302,no rules sent ro s314 add more comments here to make the code understandable https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode144 RPM/RPM.py:144: def plot_RTTflows(self): #plot graph for s302 and s314 : RTT function of nb of flows that go through s302. These flows don't go through s314 (cf topology) Have you tried to use a box plot graph as Alireza suggested ? Here is a sample code for creating a box-plot: ## Create data np.random.seed(1) # Put data to the box plot print "#####################################################" print self.collectn_1 print "#####################################################" print self.collectn_2 self.data_to_plot = [self.collectn_1,self.collectn_2] # Create a figure instance fig = plt.figure(1, figsize=(9, 6)) # Create an axes instance ax = fig.add_subplot(111) # Create the boxplot bp = ax.boxplot(self.data_to_plot) #Save the figure fig.savefig('fig1.png', bbox_inches='tight') You can find also more ideas by accessing the links: http://blog.bharatbhole.com/creating-boxplots-with-matplotlib/ https://plot.ly/matplotlib/box-plots/ https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode188 RPM/RPM.py:188: #if self.iterator_302==100: Remove commented lines(188-193) https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode226 RPM/RPM.py:226: #req = urllib2.Request(self.url, data, {'Content-Type': 'application/json', 'Content-Length': clen}) # sending the RTT to the cache why lines 226-228 are commented in? Don't we need to send POST request to Database?
Sign in to reply to this message.
Hi, here is a code review for RPM https://codereview.appspot.com/329400043/diff/80001/RPM/test.py File RPM/test.py (right): https://codereview.appspot.com/329400043/diff/80001/RPM/test.py#newcode22 RPM/test.py:22: self.nbOfRules=0 What is nbOfRules used for? https://codereview.appspot.com/329400043/diff/80001/RPM/test.py#newcode42 RPM/test.py:42: def incrRules(self): what is this function used for?
Sign in to reply to this message.
https://codereview.appspot.com/329400043/diff/80001/RPM/test.py File RPM/test.py (right): https://codereview.appspot.com/329400043/diff/80001/RPM/test.py#newcode22 RPM/test.py:22: self.nbOfRules=0 On 2017/11/29 13:58:55, 2016shana2333 wrote: > What is nbOfRules used for? nbOf rules is the number of rules to send to the switch 302 each periode of time T<<1s https://codereview.appspot.com/329400043/diff/80001/RPM/test.py#newcode42 RPM/test.py:42: def incrRules(self): On 2017/11/29 13:58:55, 2016shana2333 wrote: > what is this function used for? This function is used to to incremente the number of rules sent to s302: each 10s, we add 5*20 sets of rules
Sign in to reply to this message.
https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode113 RPM/RPM.py:113: def addRTT302_flows(self): #Computes avg RTT for s302 and s314 each time we add flows(here, 1 iperf flow added each 50s) On 2017/11/29 13:49:20, Vasileios Papageorgiou wrote: > add more comments here to make the code understandable yes, coming soon https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode144 RPM/RPM.py:144: def plot_RTTflows(self): #plot graph for s302 and s314 : RTT function of nb of flows that go through s302. These flows don't go through s314 (cf topology) On 2017/11/29 13:49:20, Vasileios Papageorgiou wrote: > Have you tried to use a box plot graph as Alireza suggested ? > Here is a sample code for creating a box-plot: > ## Create data > np.random.seed(1) > # Put data to the box plot > print "#####################################################" > print self.collectn_1 > print "#####################################################" > print self.collectn_2 > self.data_to_plot = [self.collectn_1,self.collectn_2] > # Create a figure instance > fig = plt.figure(1, figsize=(9, 6)) > # Create an axes instance > ax = fig.add_subplot(111) > # Create the boxplot > bp = ax.boxplot(self.data_to_plot) > #Save the figure > fig.savefig('fig1.png', bbox_inches='tight') > You can find also more ideas by accessing the links: > http://blog.bharatbhole.com/creating-boxplots-with-matplotlib/ > https://plot.ly/matplotlib/box-plots/ i haven't tried that but it doesn't change anything. Running my code 3 times for example is equivalent to running your code way more times, because you need to change manually the frequency of number of rules sent. In my code the frequency is incremented each 10s https://codereview.appspot.com/329400043/diff/80001/RPM/RPM.py#newcode188 RPM/RPM.py:188: #if self.iterator_302==100: On 2017/11/29 13:49:20, Vasileios Papageorgiou wrote: > Remove commented lines(188-193) yes, a clean code is comming soon
Sign in to reply to this message.
Hello! You'll find here a commented version of the RPM code. To try the code, open 2 terminals : Terminal 1:taskset 0x2 ryu-manager RPM.py simple_switch_13.py test.py --ofp-tcp-listen-port 6633 Terminal 2: taskset 0x1 sudo mn --custom stanford-topo.py --topo mytopo,s302,s301,s303,s304,s204,s209 --controller=remote,ip=127.0.0.1,port=6633 To generate traffic, add "source iperf_traffic" in the mininet CLI
Sign in to reply to this message.
Admission control test file, with updated port numbers(10001,10002,10003)
Sign in to reply to this message.
Hi Younes, Check the comments for RPM. BR, Vasilis https://codereview.appspot.com/329400043/diff/160001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/160001/RPM/RPM.py#newcode1 RPM/RPM.py:1: from ryu.base import app_manager I am adding your own draft comments here so that it would be more understandable for someone that wants to test RPM. "To try the code, open 2 terminals : Terminal 1:taskset 0x2 ryu-manager RPM.py simple_switch_13.py test.py --ofp-tcp-listen-port 6633 Terminal 2: taskset 0x1 sudo mn --custom stanford-topo.py --topo mytopo,s302,s301,s303,s304,s204,s209 --controller=remote,ip=127.0.0.1,port=6633 To generate traffic, add "source iperf_traffic" in the mininet CLI" https://codereview.appspot.com/329400043/diff/160001/RPM/RPM.py#newcode113 RPM/RPM.py:113: def addRTT302_flows(self): #Computes avg RTT for s302 and s314 each time we add flows(here, 1 iperf flow added each 30s) What do you mean by 1 iperf flow added each 30s? Does it have to with the topology/iperf_traffic test or with the topology/Admission_control test?
Sign in to reply to this message.
Hi, It seems a good script similar to what other automated test of Michael do. BR, Vasilis
Sign in to reply to this message.
https://codereview.appspot.com/329400043/diff/160001/RPM/RPM.py File RPM/RPM.py (right): https://codereview.appspot.com/329400043/diff/160001/RPM/RPM.py#newcode113 RPM/RPM.py:113: def addRTT302_flows(self): #Computes avg RTT for s302 and s314 each time we add flows(here, 1 iperf flow added each 30s) On 2017/12/06 15:24:25, Vasileios Papageorgiou wrote: > What do you mean by 1 iperf flow added each 30s? Does it have to with the > topology/iperf_traffic test or with the topology/Admission_control test? I use iperf_traffic file for this one. And in this file, you see that there is a sleep(30s) between each new iperf flow
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
