Do not include zeroed metadata fields in NXM/OXM packet-in messages.
authorBen Pfaff <blp@nicira.com>
Wed, 15 Aug 2012 17:16:49 +0000 (10:16 -0700)
committerBen Pfaff <blp@nicira.com>
Fri, 17 Aug 2012 20:20:53 +0000 (13:20 -0700)
commit42edbe39dd2dcb94ef2f43dacb3e43a01bcf1f86
treebfccabfa435612fbec633988fb3db5957655004a
parent54fa24c559938dec9be7cb64de741fe36ee83e07
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@nicira.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
include/openflow/nicira-ext.h
lib/flow.c
lib/flow.h
lib/ofp-print.c
lib/ofp-util.c
ofproto/ofproto-dpif.c
tests/ofp-print.at
tests/ofproto-dpif.at
tests/ofproto.at