On Fri, Jul 15, 2016 at 01:02:27PM +0530, deepanshu.saxen...@gmail.com wrote: > From: Deepanshu Saxena <deepanshu.saxe...@tcs.com> > > This commit implements ovs meters in kernel space using the traffic > control (tc) module of Linux kernel. TC is used to configure Traffic > Control features like Policing, Shaping etc. Ovs meters are analogous to > traffic control policing feature. The details from the meter command are > fetched and mapped to the corresponding fields of tc qdisc,class,filter > commands. > > In this commit only the "ovs-ofctl add-meter" command is translated to > the respective commands of tc. In this only the drop meter band type is > implemented. > > Tested the code using iperf3. Setup consist of ovs connected to VM using > tap interface port and another host via hardware interface. > > Meter is a switch element that can measure and control the rate of packets. > The > meter triggers a meter band if the packet rate or byte rate passing through > the meter exceeds a predefined threshold. If the meter band drops the > packet, it is called a Rate Limiter. > > Signed-off-by: Deepanshu Saxena <deepanshu.saxe...@tcs.com> > Co-authored-by : Jarno Rajahalme <jrajaha...@nicira.com>
I have some comments to add to those that you have already received. Most importantly, this doesn't seem to actually implement OpenFlow meters. Instead, it looks like it changes the qdisc on the first port, if any, that is directly output to following a meter instruction. That's not the same at all. If that's all this does, then we cannot accept it. (If users want an HTB qdisc then they can already configure.) If Jarno co-authored this, then you need to get a Signed-off-by from him too. Also you should update his email address; it is now ja...@ovn.org. When you add a new file, add a license and appropriate copyright notice. Please read CodingStyle.md and follow its rules. I don't think it's a good idea to modify ofm->fm.ofpacts in add_flow_start() because it's owned by the caller. Please do not add comments about old code. Code should explain what it is doing, not what it used to do: + /* + * This line is deleted because meter_set() is declared in + * ofproto/ofproto-provider.h but it doesnot have implementaion + * in ofproto/ofproto-dpif.c in ofproto_dpif_class. This function + * will basically set meter in the datapath. To bypass this have + * deleted the line. + */ _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
