Hi Ben, Please provide inputs on queries mentioned in the previous mail, so that I can submit revised patch (v5) of OF1.4-vacancy feature.
Thanks and Regards, Saloni Jain Tata Consultancy Services Mailto: saloni.j...@tcs.com Website: http://www.tcs.com ____________________________________________ Experience certainty. IT Services Business Solutions Consulting ____________________________________________ -----"dev" <dev-boun...@openvswitch.org> wrote: ----- To: Ben Pfaff <b...@nicira.com>, dev@openvswitch.org From: Saloni Jain Sent by: "dev" Date: 09/19/2015 03:24PM Cc: saloni.jai...@gmail.com, Shashwat Srivastava <shashwat.srivast...@tcs.com>, Deepankar Gupta <deepankar.gu...@tcs.com>, Partha Datta <partha.da...@tcs.com>, sandeep.kuma...@tcs.com Subject: Re: [ovs-dev] [PATCH v4 1/3] Implement Openflow 1.4 Vacancy Events for OFPT_TABLE_MOD. Hi Ben, Thanks for the review. I will do all the suggested changes in v5 of the patch series. Please validate my understanding for the following 2 comments: >In ovs-ofctl.c, I don't think the table-mod code handles the case where >OF1.4 or OF1.5 is enabled but the switch does not support it. This means that in ovs-ofctl.c, for table-mod, it can happen that OF1.4 and OF1.5 are supported, but the switch in table-features capabilities does not support eviction or vacancy table-config parameters. So after checking the usable version as OF1.4 and OF1.5 for table-mod in ovs-ofctl.c, table feature request should be sent to the switch from ofctl_mod_table() in order to get the supported capabilities for the given table-id and if eviction/vacancy events are supported by switch, then only table-mod config property should be set. >The syntax seems kind of odd actually. How about "vacancy(low,high)"? Parentheses - "()" and "{}" are used for command grouping in shell and will give error "syntax error:bash: syntax error near unexpected token `('", when used with any command. So in order to avoid the error, we have to use escape characters or single/double quotes around "vacancy(low,high)" in ovs-ofctl mod-table command, such that, the command looks like: ovs-ofctl -O Openflow14 mod-table br0 0 'vacancy(low,high)' I have also tried for "vacancy[low,high]", that is using square brackets [], but I am facing problem in test cases. In file ofproto.at square brackets are ignored in AT_CHECK[] and so test case for mod-table for vacancy is failing. Other possible syntax are -- vacancy:low-high or vacancy:low,high Please suggest whether to go with parentheses "vacancy(low,high)" or is there any other recommended syntax which can be used? Thanks and Regards, Saloni Jain Tata Consultancy Services Mailto: saloni.j...@tcs.com Website: http://www.tcs.com ____________________________________________ Experience certainty. IT Services Business Solutions Consulting ____________________________________________ -----"dev" <dev-boun...@openvswitch.org> wrote: ----- To: saloni.jai...@gmail.com From: Ben Pfaff Sent by: "dev" Date: 09/18/2015 04:00AM Cc: dev@openvswitch.org, Shashwat Srivastava <shashwat.srivast...@tcs.com>, deepankar.gu...@tcs.com, partha.da...@tcs.com, Sandeep Kumar <sandeep.kuma...@tcs.com> Subject: Re: [ovs-dev] [PATCH v4 1/3] Implement Openflow 1.4 Vacancy Events for OFPT_TABLE_MOD. On Wed, Sep 09, 2015 at 01:12:04PM +0530, saloni.jai...@gmail.com wrote: > From: Saloni Jain <saloni.j...@tcs.com> > > OpenFlow 1.4 introduces the ability to turn on vacancy events with an > OFPT_TABLE_MOD message specifying OFPTC_VACANCY_EVENTS. This commit adds > support for the new feature in ovs-ofctl mod-table. > As per the openflow specification-1.4, vacancy event adds a mechanism > enabling the controller to get an early warning based on capacity > threshold chosen by the controller. > > With this commit, vacancy events can be configured as: > ovs-ofctl -O OpenFlow14 mod-table <bridge> <table> vacancy-<range> > The syntax of <range> as <low..high>. > <range> specify vacancy threshold values, vacancy down and vacancy up. > > To disable vacancy events, following command should be given: > ovs-ofctl -O OpenFlow14 mod-table <bridge> <table> novacancy > > Signed-off-by: Saloni Jain <saloni.j...@tcs.com> > Co-authored-by: Shashwat Srivastava <shashwat.srivast...@tcs.com> > Signed-off-by: Shashwat Srivastava <shashwat.srivast...@tcs.com> > Co-authored-by: Sandeep Kumar <sandeep.kuma...@tcs.com> > Signed-off-by: Sandeep Kumar <sandeep.kuma...@tcs.com> Thanks for the patch! This needs to update the documentation for ovs-ofctl to mention the new feature and describe the syntax. The syntax seems kind of odd actually. How about "vacancy(low,high)"? I think that the format output by ofp_print_table_mod() should match that accepted by ofp-parse instead of being completely different. One of the patches in this series should update NEWS to mention the new feature. The 'vacancy' member in struct oftable appears to have exactly two values: 0 and OFPTC14_VACANCY_EVENTS. Why is it an "unsigned int" instead of a Boolean (e.g. "bool vacancy_enabled")? I think that parse_table_mod_vacancy_property() should report an error for a "percentage" that exceeds 100. In a table_mod the 'vacancy' member of struct ofp14_table_mod_prop_vacancy is supposed to be always zero, so I don't see why ofputil_encode_table_mod() copies this in from its argument instead of leaving it zeroed. In ovs-ofctl.c, I don't think the table-mod code handles the case where OF1.4 or OF1.5 is enabled but the switch does not support it. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev =====-----=====-----===== Notice: The information contained in this e-mail message and/or attachments to it may contain confidential or privileged information. If you are not the intended recipient, any dissemination, use, review, distribution, printing or copying of the information contained in this e-mail message and/or attachments to it are strictly prohibited. If you have received this communication in error, please notify us by reply e-mail or telephone and immediately and permanently delete the message and any attachments. Thank you _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
