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.
>
>> diff --git a/utilities/ovs-dpctl.8.in b/utilities/ovs-dpctl.8.in
>> index 5c01570..c8345a5 100644
>> --- a/utilities/ovs-dpctl.8.in
>> +++ b/utilities/ovs-dpctl.8.in
>> @@ -118,11 +118,19 @@ exactly one datapath exists, in which case that
>> datapath is the
>> default. When multiple datapaths exist, then a datapath name is
>> required.
>> .
>> -.IP "[\fB\-m \fR| \fB\-\-more\fR] \fBdump\-flows\fR [\fIdp\fR]"
>
> The = should probably be bold too here (it's a literal, right?) and
> later on in various places too:
I will correct this.
>> +.IP "[\fB\-m \fR| \fB\-\-more\fR] \fBdump\-flows\fR [\fIdp\fR]
>> [\fBfilter\fR=\fIfilter\fR]"
>> Prints to the console all flow entries in datapath \fIdp\fR's flow
>> table. Without \fB\-m\fR or \fB\-\-more\fR, output omits match fields
>> that a flow wildcards entirely; with \fB\-m\fR or \fB\-\-more\fR,
>> output includes all wildcarded fields.
>> +.IP
>> +If \fBfilter\fR=\fIfilter\fR is specified, only displays the flows
>> +that match the \fIfilter\fR. \fIfilter\fR is a flow in the form similiar
>> +to that accepted by \fBovs\-ofctl\fR(8)'s \fBadd\-flow\fR command. (This is
>> +not an OpenFlow flow: besides other differences, it never contains
>> wildcards.)
>> +The \fIfilter\fR is also useful to match wildcarded fields in the datapath
>> +flow. As an example, \fBfilter\fR='\fBtcp,tp_src=100\fR' will match the
>> +datapath flow containing '\fBtcp(src=80/0xff00,dst=8080/0xff)\fR'.
>
> ...
>
>> + if (argc == 1) {
>> + name = get_one_dp();
>> + } else if (argc == 2) {
>> + if (!strncmp(argv[1], "filter=", 7)) {
>
> Can I get a space on each side of the + here and below?
>
>> + filter = xstrdup(argv[1]+7);
>> + name = get_one_dp();
>> + } else {
>> + name = xstrdup(argv[1]);
>> + }
>> + } else {
>> + name = xstrdup(argv[1]);
>
> This is not going to behave well if argv[2] is shorter than 7
> characters:
Oops.
>> + filter = xstrdup(argv[2]+7);
>> + }
>
> Something like this might work:
>
> if (argc > 1 && !strncmp(argv[argc - 1], "filter=", 7)) {
> filter = xstrdup(argv[--argc] + 7);
> }
> name = (argc == 2) ? xstrdup(argv[1]) : get_one_dp();
I will use this.
>
> Thanks,
>
> Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
