[ovs-dev] [PATCH V6 09/18] dpctl: Support dump-flows filters "dpdk" and "partially-offloaded"

Eli Britstein elibr at mellanox.com
Wed Jan 8 17:25:43 UTC 2020


On 1/8/2020 2:17 PM, Ilya Maximets wrote:
> On 19.12.2019 12:54, Eli Britstein wrote:
>> Flows that are offloaded via DPDK can be partially offloaded (matches
>> only) or fully offloaded (matches and actions). Set partially offloaded
>> display to (offloaded=partial, dp:ovs), and fully offloaded to
>> (offloaded=yes, dp:dpdk). Also support filter types "dpdk" and
>> "partially-offloaded".
>>
>> Signed-off-by: Eli Britstein <elibr at mellanox.com>
>> ---
>>   NEWS          |  3 ++
>>   lib/dpctl.c   | 97 ++++++++++++++++++++++++++++++++++++++++++++---------------
>>   lib/dpctl.man |  2 ++
>>   3 files changed, 78 insertions(+), 24 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 2acde3fe8..85c4a4812 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -26,6 +26,9 @@ Post-v2.12.0
>>        * DPDK ring ports (dpdkr) are deprecated and will be removed in next
>>          releases.
>>        * Add support for DPDK 19.11.
>> +   - 'ovs-appctl dpctl/dump-flows' can now show offloaded=partial for
>> +     partially offloaded flows, dp:dpdk for fully offloaded by dpdk, and
>> +     type filter supports new filters: "dpdk" and "partially-offloaded".
>>   
>>   v2.12.0 - 03 Sep 2019
>>   ---------------------
>> diff --git a/lib/dpctl.c b/lib/dpctl.c
>> index 0ee64718c..387f61ed4 100644
>> --- a/lib/dpctl.c
>> +++ b/lib/dpctl.c
>> @@ -818,7 +818,11 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports,
>>   
>>       dpif_flow_stats_format(&f->stats, ds);
>>       if (dpctl_p->verbosity && f->attrs.offloaded) {
>> -        ds_put_cstr(ds, ", offloaded:yes");
>> +        if (f->attrs.dp_layer && !strcmp(f->attrs.dp_layer, "ovs")) {
>> +            ds_put_cstr(ds, ", offloaded:partial");
>> +        } else {
>> +            ds_put_cstr(ds, ", offloaded:yes");
>> +        }
>>       }
>>       if (dpctl_p->verbosity && f->attrs.dp_layer) {
>>           ds_put_format(ds, ", dp:%s", f->attrs.dp_layer);
>> @@ -827,20 +831,30 @@ format_dpif_flow(struct ds *ds, const struct dpif_flow *f, struct hmap *ports,
>>       format_odp_actions(ds, f->actions, f->actions_len, ports);
>>   }
>>   
>> +enum dp_type {
>> +    DP_TYPE_ANY,
>> +    DP_TYPE_OVS,
>> +    DP_TYPE_TC,
>> +    DP_TYPE_DPDK,
>> +};
>> +
>> +enum ol_type {
>> +    OL_TYPE_ANY,
>> +    OL_TYPE_NO,
>> +    OL_TYPE_YES,
>> +    OL_TYPE_PARTIAL,
>> +};
>> +
>>   struct dump_types {
>> -    bool ovs;
>> -    bool tc;
>> -    bool offloaded;
>> -    bool non_offloaded;
>> +    enum dp_type dp_type;
>> +    enum ol_type ol_type;
>>   };
>>   
>>   static void
>>   enable_all_dump_types(struct dump_types *dump_types)
>>   {
>> -    dump_types->ovs = true;
>> -    dump_types->tc = true;
>> -    dump_types->offloaded = true;
>> -    dump_types->non_offloaded = true;
>> +    dump_types->dp_type = DP_TYPE_ANY;
>> +    dump_types->ol_type = OL_TYPE_ANY;
>>   }
>>   
>>   static int
>> @@ -862,13 +876,17 @@ populate_dump_types(char *types_list, struct dump_types *dump_types,
>>           current_type[type_len] = '\0';
>>   
>>           if (!strcmp(current_type, "ovs")) {
>> -            dump_types->ovs = true;
>> +            dump_types->dp_type = DP_TYPE_OVS;
>>           } else if (!strcmp(current_type, "tc")) {
>> -            dump_types->tc = true;
>> +            dump_types->dp_type = DP_TYPE_TC;
>> +        } else if (!strcmp(current_type, "dpdk")) {
>> +            dump_types->dp_type = DP_TYPE_DPDK;
>>           } else if (!strcmp(current_type, "offloaded")) {
>> -            dump_types->offloaded = true;
>> +            dump_types->ol_type = OL_TYPE_YES;
>>           } else if (!strcmp(current_type, "non-offloaded")) {
>> -            dump_types->non_offloaded = true;
>> +            dump_types->ol_type = OL_TYPE_NO;
>> +        } else if (!strcmp(current_type, "partially-offloaded")) {
>> +            dump_types->ol_type = OL_TYPE_PARTIAL;
>>           } else if (!strcmp(current_type, "all")) {
>>               enable_all_dump_types(dump_types);
>>           } else {
>> @@ -884,30 +902,61 @@ static void
>>   determine_dpif_flow_dump_types(struct dump_types *dump_types,
>>                                  struct dpif_flow_dump_types *dpif_dump_types)
>>   {
>> -    dpif_dump_types->ovs_flows = dump_types->ovs || dump_types->non_offloaded;
>> -    dpif_dump_types->netdev_flows = dump_types->tc || dump_types->offloaded
>> -                                    || dump_types->non_offloaded;
>> +    dpif_dump_types->ovs_flows = dump_types->dp_type == DP_TYPE_OVS;
>> +    dpif_dump_types->netdev_flows = (dump_types->dp_type == DP_TYPE_TC ||
>> +                                     dump_types->dp_type == DP_TYPE_DPDK);
>>   }
> Above code doesn't handle DP_TYPE_ANY.  This mostly breaks dump-flows
> command if no type specified.
Correct. I already fixed it today. That was the first issue with make 
check-offloads.
>
> Some more issues:  I think this patch will not handle multiple flow
> types correctly.  For example, "dump-flows --type=tc,dpdk" should
> dump "flows that are in TC, but not in HW" + "flows that are in HW via
> DPDK". So, it should not dump flows that handled purely by OVS or
> offloaded to HW via TC.  I believe, this case will not work correctly
> with current implementation of this patch.

My understanding of this "type" parameter was that it is a kind of a 
filter, that should narrow down the flows dumped, so the condition is 
*AND* and not *OR*.

With "--type=tc,dpdk", it won't dump anything as a flow can't be both.

The second issue with make check-offloads was that it checked 
"tc,offloaded", and got nothing, as those flows were "tc", but not 
offloaded.

I think that's more correct behavior, so I changed the test.

Please comment your thoughts before I submit v7.

>
>>   
>>   static bool
>> -flow_passes_type_filter(const struct dpif_flow *f,
>> -                        struct dump_types *dump_types)
>> +flow_passes_dp_filter(const struct dpif_flow *f,
>> +                      struct dump_types *dump_types)
>>   {
>> -    if (dump_types->ovs && !strcmp(f->attrs.dp_layer, "ovs")) {
>> +    if (dump_types->dp_type == DP_TYPE_ANY) {
>>           return true;
>>       }
>> -    if (dump_types->tc && !strcmp(f->attrs.dp_layer, "tc")) {
>> -        return true;
>> +    if (dump_types->dp_type == DP_TYPE_OVS) {
>> +        return !strcmp(f->attrs.dp_layer, "ovs");
>>       }
>> -    if (dump_types->offloaded && f->attrs.offloaded) {
>> -        return true;
>> +    if (dump_types->dp_type == DP_TYPE_TC) {
>> +        return !strcmp(f->attrs.dp_layer, "tc");
>>       }
>> -    if (dump_types->non_offloaded && !(f->attrs.offloaded)) {
>> +    if (dump_types->dp_type == DP_TYPE_DPDK) {
>> +        return !strcmp(f->attrs.dp_layer, "dpdk");
>> +    }
>> +    /* should never get here. */
>> +    return false;
>> +}
>> +
>> +static bool
>> +flow_passes_ol_filter(const struct dpif_flow *f,
>> +                      struct dump_types *dump_types)
>> +{
>> +    if (dump_types->ol_type == OL_TYPE_ANY) {
>>           return true;
>>       }
>> +    if (dump_types->ol_type == OL_TYPE_NO) {
>> +        return !f->attrs.offloaded;
>> +    }
>> +    if (dump_types->ol_type == OL_TYPE_YES &&
>> +        strcmp(f->attrs.dp_layer, "ovs")) {
>> +        return f->attrs.offloaded;
>> +    }
>> +    if (dump_types->ol_type == OL_TYPE_PARTIAL &&
>> +        !strcmp(f->attrs.dp_layer, "ovs")) {
>> +        return f->attrs.offloaded;
>> +    }
>> +    /* should never get here. */
>>       return false;
>>   }
>>   
>> +static bool
>> +flow_passes_type_filter(const struct dpif_flow *f,
>> +                        struct dump_types *dump_types)
>> +{
>> +    return flow_passes_dp_filter(f, dump_types) &&
>> +           flow_passes_ol_filter(f, dump_types);
>> +}
>> +
>>   static struct hmap *
>>   dpctl_get_portno_names(struct dpif *dpif, const struct dpctl_params *dpctl_p)
>>   {
>> diff --git a/lib/dpctl.man b/lib/dpctl.man
>> index 090c3b960..727d1f7be 100644
>> --- a/lib/dpctl.man
>> +++ b/lib/dpctl.man
>> @@ -124,8 +124,10 @@ This option supported only for \fBovs\-appctl dpctl/dump\-flows\fR.
>>   .
>>      \fBovs\fR - displays flows handled in the ovs dp
>>      \fBtc\fR - displays flows handled in the tc dp
>> +   \fBdpdk\fR - displays flows fully offloaded by dpdk
>>      \fBoffloaded\fR - displays flows offloaded to the HW
>>      \fBnon-offloaded\fR - displays flows not offloaded to the HW
>> +   \fBpartially-offloaded\fR - displays flows where only part of their proccessing is done in HW
>>      \fBall\fR - displays all the types of flows
>>   .IP
>>   By default all the types of flows are displayed.
>>


More information about the dev mailing list