[ovs-dev] [PATCH v2] ofp-actions: Implement writing to metadata field

Ben Pfaff blp at nicira.com
Fri Jul 13 04:01:46 UTC 2012


On Fri, Jul 13, 2012 at 02:48:49PM +1200, Joe Stringer wrote:
> On 13 July 2012 12:02, Ben Pfaff <blp at nicira.com> wrote:
> > Suppose you fix that; it should not be hard (just remove an "else"
> > keyword, I think).  Then, second, the semantics implemented here will
> > surprise users, because if ofpacts are translated to OF1.0 then each
> > OFPIT_WRITE_METADATA will be executed one at a time at the point where
> > it appears in the actions, but if they are translated to OF1.1 then only
> > a single OFPIT_WRITE_METADATA will be processed, after all other
> > actions.  I'd rather avoid that.  One idea: report an error if
> > "write_metadata" is specified more than once or if it is not specified
> > as the last action.
> 
> Do we want to just limit a maximum of one ofpact_metadata per ofpacts?
> 
> So, with the above patch:-
> When OFPIT_WRITE_METADATA is converted to ofpacts, an error is thrown
> if there is more than one. The conversion is done last, so we don't
> care where the OFPIT_WRITE_METADATA is placed within the message; it
> is parsed last, and added to the end of ofpacts. When the ofpacts are
> converted back to OF1.1, we ignore the NXAST translation case and
> always write OFPIT_WRITE_METADATA last. If we only have a maximum of
> one ofpact_metadata in our ofpacts, then this is all fine.

Yes, all of that makes sense.

> The trouble is with NXAST_WRITE_METADATA; Parsing to ofpacts may
> currently cause multiple metadata_ofpacts to appear internally, but we
> don't want this. If we can restrict this to one entry, then the
> reverse process (ofpacts->NX), just needs to ensure that the
> NXAST_WRITE_METADATA action is the last to appear in the actions list.

Yes, I think that we should restrict parsing NXAST_WRITE_METADATA to one
entry and only as the final entry.

> Is this the preferred approach? Are there other examples which may
> cause multiple instances of the ofpact_metadata to be stored in
> ofpacts?

There are only two important paths into ofpacts: parsing of binary
instructions and actions, and parsing of text versions of instructions
and actions.  We'd want to make sure that both of these paths enforce
the constraint.



More information about the dev mailing list