[ovs-dev] [PATCH v2 2/2] Set datapath mask bits when setting a flow field.

Jarno Rajahalme jrajahalme at nicira.com
Tue Oct 15 17:23:02 UTC 2013


On Oct 15, 2013, at 9:52 AM, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, Oct 15, 2013 at 09:19:52AM -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
> 
> "explicitly"
> 

Thanks.

>> 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 <jrajahalme at nicira.com>
> 
> I understand why this adds mf_force_prereqs(), but I don't understand
> why it removes the mf_write_subfield_flow() call from
> nxm_execute_reg_move().  Can you explain?
> 

I placed the equivalent mf_set_flow_value() call in the mf_force_prereqs(), which I now see should be renamed.

Note that it is equivalent in the sense that masking only some bits vs. all the bits of the field usually makes no difference, as the ODP commit will anyway set them all if there is an actual change, and for the (rare?) case where the set/load did not change anything, it is better not to fragment the datapath mega-flow masks into too many different kinds of masks by masking the sub-field bits only.

Maybe I got too carried away noticing that there was a prototype for mf_force_prereqs() that was actually needed but not implemented.. How about mf_mask_field_and_prereqs() for a better name?

  Jarno




More information about the dev mailing list