[ovs-dev] [PATCH v4 1/3] Implement Openflow 1.4 Vacancy Events for OFPT_TABLE_MOD.

[Ben Pfaff]

On Wed, Sep 09, 2015 at 01:12:04PM +0530, saloni.jain12 at gmail.com wrote:
> From: Saloni Jain <saloni.jain at 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.jain at tcs.com>
> Co-authored-by: Shashwat Srivastava <shashwat.srivastava at tcs.com>
> Signed-off-by: Shashwat Srivastava <shashwat.srivastava at tcs.com>
> Co-authored-by: Sandeep Kumar <sandeep.kumar16 at tcs.com>
> Signed-off-by: Sandeep Kumar <sandeep.kumar16 at 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.