[ovs-git] Open vSwitch: Do not include zeroed metadata fields in NXM/OXM packet-in messages. (master)

[dev at openvswitch.org]

This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Open vSwitch".

The branch, master has been updated
       via  42edbe39dd2dcb94ef2f43dacb3e43a01bcf1f86 (commit)
      from  54fa24c559938dec9be7cb64de741fe36ee83e07 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 42edbe39dd2dcb94ef2f43dacb3e43a01bcf1f86
Diffs: http://openvswitch.org/cgi-bin/gitweb.cgi?p=openvswitch;a=commitdiff;h=42edbe39dd2dcb94ef2f43dacb3e43a01bcf1f86
Author: Ben Pfaff <blp at nicira.com>
		
Do not include zeroed metadata fields in NXM/OXM packet-in messages.
		
NXM and OpenFlow 1.2+ allow including the values of arbitrary flow metadata
in "packet-in" messages.  Open vSwitch has until now always included all
the values of the metadata fields that it implements in NXT_PACKET_IN
messages.

However, this has at least two disadvantages:

    - Most of the metadata fields tend to be zero most of the time, which
      wastes space in the message.

    - It means that controllers must be very liberal about accepting
      fields that they know nothing about in packet-in messages, since any
      switch upgrade could cause new fields to appear even if the
      controller does nothing to give them nonzero values.  (Controllers
      have to be prepared to tolerate unknown fields in any case, but this
      property makes unknown fields more likely to appear than otherwise.)

This commit changes Open vSwitch so that metadata fields whose values are
zero are not reported in packet-ins, fixing both problems.  (This is
explicitly allowed by OpenFlow 1.2+.)

This commit mainly fixes a sort of internal conceptual dissonance centering
around struct flow_metadata.  This structure is supposed to report the
metadata for a given flow.  If you look at a flow, it has particular
metadata values; it doesn't have masks, and the idea of a mask for a
particular flow doesn't really make sense.  However, struct flow_metadata
did have masks.  This led to internal confusion; one can see this in, for
example, the following code removed by this commit in ofproto-dpif.c to
handle misses in the OpenFlow flow table:

    /* Registers aren't meaningful on a miss. */
    memset(pin.fmd.reg_masks, 0, sizeof pin.fmd.reg_masks);

What this code was really trying to say is that on a flow miss, the
registers are zero, so they shouldn't be included in the packet-in message.
It did manage to omit the registers, by marking them as "wild", but it is
conceptually more correct to simply omit them because they are zero (and
that's one effect of this commit).

Bug #12968.
Reported-by: Igor Ganichev <iganichev at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>


-----------------------------------------------------------------------

Summary of changes:
 include/openflow/nicira-ext.h |   37 +++++++++++++++++++++++--------------
 lib/flow.c                    |    6 ------
 lib/flow.h                    |   12 ++----------
 lib/ofp-print.c               |   15 +++------------
 lib/ofp-util.c                |   21 +++++++++------------
 ofproto/ofproto-dpif.c        |    3 ---
 tests/ofp-print.at            |    4 ++--
 tests/ofproto-dpif.at         |   38 +++++++++++++++++++-------------------
 tests/ofproto.at              |    2 +-
 9 files changed, 59 insertions(+), 79 deletions(-)


hooks/post-receive
-- 
Open vSwitch