On Fri, Jun 19, 2015 at 04:13:17PM -0700, Jesse Gross wrote:
> This adds support for receiving variable length fields encoded in
> NXM/OXM and mapping them into OVS internal structures. In order
> for this to make sense, we need to define some semantics:
>
> There are three lengths that matter in this process: the maximum
> size of the field (represented as the existing mf->n_bytes), the
> size of the field in the incoming NXM (given by the length in the
> NXM header), and the currently configured length of the field
> (defined by the consumer of the field and outside the scope of
> this patch).
>
> Fields are modeled as being their maximum length and have the
> characteristics expected by exsiting code (i.e. exact match fields
> have masks that are all 1's for the whole field, etc.). Incoming
> NXMs are stored in the field in the least significant bits. If
> the NXM length is larger than the field, is is truncated, if it
> is smaller it is zero-extended. When the field is consumed, the
> component that needs data picks the configured length out of the
> generated field.
>
> In most cases, the configured and NXM lengths will be equal and
> these edge cases do not matter. However, since we cannot easily
> enforce that the lengths match (and might not even know what the
> right length is, such as in the case of a string parsed by
> ovs-ofctl), these semantics should provide deterministic results
> that are easy to understand and not require most existing code
> to be aware of variable length fields.
>
> Signed-off-by: Jesse Gross <je...@nicira.com>
I think that this changes behavior in a corner case. Previously, if
nx_pull_entry__() were passed a null 'field' parameter, then
nx_pull_header__() would not check for a field at all and would never
return OFPERR_OFPBMC_BAD_FIELD. Now, nx_pull_header__() always checks
for a field, and although nx_pull_entry__() initially ignores
OFPERR_OFPBMC_BAD_FIELD, it still eventually passes that error along to
its caller where it previously returned 0.
I don't know whether that's a big deal but I do recall trying to get
this exactly right before. The fix seems simple, just change the last
bit of the function to:
if (field_) {
*field_ = field;
return header_error;
}
return 0;
There's cut-and-paste copy code in this function, maybe a helper would
be nice.
Acked-by: Ben Pfaff <b...@nicira.com>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
