mirror of
https://github.com/openvswitch/ovs
synced 2025-08-31 06:15:47 +00:00
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>
This commit is contained in:
@@ -497,14 +497,8 @@ flow_get_metadata(const struct flow *flow, struct flow_metadata *fmd)
|
||||
BUILD_ASSERT_DECL(FLOW_WC_SEQ == 14);
|
||||
|
||||
fmd->tun_id = flow->tun_id;
|
||||
fmd->tun_id_mask = htonll(UINT64_MAX);
|
||||
|
||||
fmd->metadata = flow->metadata;
|
||||
fmd->metadata_mask = htonll(UINT64_MAX);
|
||||
|
||||
memcpy(fmd->regs, flow->regs, sizeof fmd->regs);
|
||||
memset(fmd->reg_masks, 0xff, sizeof fmd->reg_masks);
|
||||
|
||||
fmd->in_port = flow->in_port;
|
||||
}
|
||||
|
||||
|
Reference in New Issue
Block a user