On Mon, Jun 1, 2015 at 2:55 PM, Ben Pfaff <b...@nicira.com> wrote: > On Mon, Jun 01, 2015 at 01:49:30PM -0700, Jesse Gross wrote: >> We have a special flow_metadata structure to represent the parts >> of a packet that aren't carried in the payload itself. This is >> used in the case where we need to send the packet as a Packet In >> to an OpenFlow controller. This is a subset of the more general >> struct flow. >> >> In practice, almost all operations we do on this structure involve >> converting it to or from a match or have code that is the same as >> a match. Serialization to NXM and back is done as a match. There >> is special flow_metadata formatting code that is almost identical >> to match formatting. >> >> The uses for struct flow_metadata aren't performance critical >> when it comes to memory, so we can save quite a bit of code by >> just using a match structure directly instead. In addition, as >> metadata increases and becomes more complex (Geneve options require >> some special handling beyond just additional fields), using the >> match structure means we only have to do this work in one place. >> >> Signed-off-by: Jesse Gross <je...@nicira.com> > > Most of this rationale makes sense to me, except that I'm surprised > that struct flow_metadata (which doesn't have masks) gets converted to > struct match (which does). Maybe I should just dig into the code, but > is there some big reason why this is the case?
The main reason is just that struct match is the more general superset and the encode/decoding/formatting code operates on that to be able to handle everything so it's cleaner to just use a match even if it is slightly less memory efficient. However, we actually do use the wildcards in a limited way to indicate which fields are significant and should be encoded or printed. Even though struct flow_metadata didn't have wildcards, it generated some when actually doing anything by checking fields to see if they are zero. More future looking, I'm planning on adding some additional metadata to struct match for Geneve to track the allocation of options when they are used for temporary operations like ovs-ofctl or packet metadata. This has more complex conversion logic than what is currently there and so I was trying to avoid it - that was the initial impetus for the patch but I figured that since it is a cleanup I might as well send it out independently. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
