On Wed, Oct 19, 2011 at 2:07 PM, Ben Pfaff <[email protected]> wrote:
> I think that the intention is that OVS_ACTION_ATTR_{SET,PUSH} contain
> exactly one nested attribute, but the comment on it doesn't say that.
> It's common to support a bunch of nested attributes, and so I could
> see someone trying to set several keys in a single
> OVS_ACTION_ATTR_SET, so it's probably a good idea to mention that only
> a single field can be set with a single OVS_ACTION_ATTR_{SET,PUSH}.

We should probably also enforce this in the kernel validation function.

It may also be important to check for unknown attributes in
validate_sample() as well.  I could see us adding an "else" attribute
and older versions shouldn't just silently ignore that.  The userspace
action is similarly forgiving but since it doesn't actually change the
packet that's probably a good thing.

> In dp_netdev_set_dl(), I don't see a benefit to doing the
> comparisons.  I think that we can just copy in the new Ethernet source
> and destination directly.

I think this is probably true in the kernel as well.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to