On Oct 15, 2013, at 8:53 AM, Ben Pfaff <[email protected]> wrote:

> On Mon, Oct 14, 2013 at 03:57:35PM -0700, Jarno Rajahalme wrote:
>> Since at the datapath interface we do not have set actions for
>> individual fields, but larger sets of fields for a given protocol
>> layer, the set action will in practice only ever apply to exactly
>> matched flows for the given protocol layer.  For example, if the
>> reg_load changes the IP TTL, the corresponding datapath action will
>> rewrite also the IP addresses and TOS byte.  Since these other field
>> values may not be explicity set, they depend on the incoming flow field
>> values, and are hence all of them are set in the wildcards masks, when
>> the action is committed to the datapath.  For the rare case, where the
>> reg_load action does not actually change the value, and no other flow
>> field values are set (or loaded), the datapath action is skipped, and
>> no mask bits are set.  Such a datapath flow should, however, be
>> dependent on the specific field value, so the corresponding wildcard
>> mask bits must be set, lest the datapath flow be applied to packets
>> containing some other value in the field and the field value remain
>> unchanged regardless of the incoming value.
>> 
>> Signed-off-by: Jarno Rajahalme <[email protected]>
> 
> If I NXAST_REG_LOAD to, say, NXM_OF_TCP_SRC, then I agree that we need
> to un-wildcard the prerequisites (why not dl_type? I guess it is
> redundant, but it would make mf_force_prereqs() make more sense), and so
> this fixes a problem.  But I don't think that it is sufficient, because
> if we actually emit an ODP action to set the TCP source port, then that
> action will also, unavoidably, set the TCP destination port too, and so
> we need to un-wildcard that.

> I guess that commit_odp_actions() and the
> functions it calls are the most logical place to do that, although
> currently they don't take a wc argument.

Actually they do, so the ODP part is already working as you describe above.

I'll send a v2 to address your comments on the first patch.

  Jarno


_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to