[ovs-dev] [PATCH] ovs-dpctl: Add a 'filter' option to match wildcarded 'dump-flows'.

[Gurucharan Shetty]

On Wed, Oct 16, 2013 at 3:57 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Wed, Oct 16, 2013 at 11:22:31AM -0700, Gurucharan Shetty wrote:
>> On Tue, Oct 15, 2013 at 5:01 PM, Ben Pfaff <blp at 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 <gshetty at 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?
Okay. I think only the MFF_DL_VLAN is the problem.
I sent a v2 with a wrapper. I am not sure whether the name of the
wrapper function
could be better.