[ovs-dev] [PATCH 07/11] openflow: Table maintenance commands for Geneve options.

[Ben Pfaff]

On Fri, Jun 19, 2015 at 04:13:21PM -0700, Jesse Gross wrote:
> In order to work with Geneve options, we need to maintain a mapping
> table between an option (defined by <class, type, length>) and
> an NXM field that can be operated on for the purposes of matches,
> actions, etc. This mapping must be explicitly specified by the
> user.
> 
> Conceptually, this table could be communicated using either OpenFlow
> or OVSDB. Using OVSDB requires less code and definition of extensions
> than OpenFlow but introduces the possibility that mapping table
> updates and flow modifications are desynchronized from each other.
> This is dangerous because the mapping table signifcantly impacts the
> way that flows using Geneve options are installed and processed by
> OVS. Therefore, the mapping table is maintained using OpenFlow commands
> instead, which opens the possibility of using synchronization between
> table changes and flow modifications through barriers, bundles, etc.
> 
> There are two primary groups of OpenFlow messages that are introduced
> as Nicira extensions: modification commands (add, modify, delete,
> clear mappings) and table status request/reply to dump the current
> table along with switch information.
> 
> Note that mappings should not be changed while they are in active use by
> a flow. The result of doing so is undefined.
> 
> This only adds the OpenFlow infrastructure but doesn't actually
> do anything with the information yet after the messages have been
> decoded.
> 
> Signed-off-by: Jesse Gross <jesse at nicira.com>

This is quite clean, thank you!

In parse_ofp_geneve_table_mod_str(), I'd prefer to use %i instead of %x,
because one of my long-term goals for OVS is to switch away from this
awful scanf() stuff to a proper parser and having hex numbers without 0x
prefixes will make that a lot harder.

This ovs-ofctl syntax is going to trip up anyone who isn't careful about
shell quoting, both for {a,b,c} and for the > in the syntax.  I guess
that's OK but the example in the ovs-ofctl manpage should really show
single or double quotes in my opinion.

I think decode_geneve_table_mappings() has a memory leak in the error
case.  You could fix it by doing the list_push_back() immediately
instead of only following validation.

It would be kind to include in the documentation the minimum required
OVS version and that this is an Open vSwitch extension.

Acked-by: Ben Pfaff <blp at nicira.com>