On Tue, Oct 15, 2013 at 10:23:02AM -0700, Jarno Rajahalme wrote: > > On Oct 15, 2013, at 9:52 AM, Ben Pfaff <[email protected]> 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 <[email protected]> > > > > 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.
Oh. I'm really not paying attention here, am I? I keep thinking that mf_force_prereqs() has the effect that I originally meant it to have, which is different from the one that you defined it to have. For my own sanity, renaming it might be a good idea. > 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? Yes, that's better. We should probably remove the unimplemented prototype. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
