On 08.01.2020 18:25, Eli Britstein wrote:
>
> 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 <el...@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.
The test is correct. And condition should be *OR*. Most of the
options are mutually exclusive, so *AND* doesn't make much sense here.
'tc' stands for flows that handled by TC, i.e. not offloaded to HW.
'offloaded' stands for flows offloaded to HW.
So, 'tc,offloaded' - flows that are not in OVS and handled by TC or HW.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
