On Wed, Oct 16, 2013 at 11:22:31AM -0700, Gurucharan Shetty wrote: > On Tue, Oct 15, 2013 at 5:01 PM, Ben Pfaff <b...@nicira.com> wrote: > > On Mon, Oct 14, 2013 at 08:55:35AM -0700, Gurucharan Shetty wrote: > >> With mega-flows, many flows in the kernel datapath are wildcarded. > >> For someone that is debugging a system and wants to find a particular > >> flow and its actions, it is a little hard to zero-in on the flow > >> because some fields are wildcarded. > >> > >> With the filter='$filter' option, we can now filter on the o/p > >> of 'ovs-dpctl dump-flows'. > >> > >> Signed-off-by: Gurucharan Shetty <gshe...@nicira.com> > > > > I'd like to avoid adding another function that has to be modified > > every time we add another field. I think that we can replaced > > mf_set_flow_mask() by these two lines currently in > > mf_mask_field_and_prereqs(): > > > > static const union mf_value exact_match_mask = > > MF_EXACT_MASK_INITIALIZER; > > > > mf_set_flow_value(mf, &exact_match_mask, mask); > > > > and then make mf_mask_field_and_prereqs() call mf_set_flow_mask(). > > (But mf_mask_field() might be a better name for the new function.) > > This looks better. But I think there is a problem. > Consider the following code in mf_set_flow_value() > > case MFF_DL_VLAN: > flow_set_dl_vlan(flow, value->be16); > break; > > In flow_set_dl_vlan(), when vid is all 1's, flow->vlan_tci is set as > zero. That breaks my vlan filtering.
That's distressing, but I see why we want different behaviors. Most of the cases will act exactly the same for each field. I'd like to avoid having two functions with very similar code in meta-flow.c. Do you think that you could implement one of these functions as a wrapper for the other, handling exceptional cases specially in the wrapper? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
