Re: [dpdk-dev] [RFC PATCH] ethdev: add support for testpmd-compliant flow rule dumping

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

Hi Ori,

On 6/2/21 4:32 PM, Ori Kam wrote:
> Hi Ivan,
> 
>> -----Original Message-----
>> From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
>>
>> Hi Ori,
>>
>> Your review efforts are much appreciated. I understand your concern
>> about the partial item/action coverage, but there are some points to be
>> considered when addressing it:
>> - It's anyway hardly possible to use the printed flow directly in
>> testpmd if it contains "opaque", or "PMD-specific", items/actions in
>> terms of the tunnel offload model. These items/actions have to be
>> omitted when printing the flow, and their absence in the resulting
>> string means that copy/pasting the flow to testpmd isn't helpful in this
>> particular case.
> I fully agree with you that some of the rules can't be printed. That is why.
> I'm not sure having partial solution is the way to go.

Sorry, I disagree that possibility to cover 99% and
impossibility to cover just 1% is the reason to discard.

> If OVS for example cares about
> some of the item/action, maybe this log should be on their part.

OvS does and as far as I can see already has bugs there.
Of course, nobody says that it is testpmd-complient format,
but it definitely looks so.

Anyway, it sounds strange do duplicate the functionality in
many-many DPDK apps. Of course, it removes the headache
from DPDK maintainers, but it is hardly friendly to DPDK
community in general.

>> - There's action ENCAP which also can't be fully represented by the tool
>> in question, simply because it has no parameters. In tespmd, one first
>> has to issue "set vxlan" command to configure the encap. header, whilst
>> "vxlan" token in the flow rule string just refers to the previously set
>> encap. parameters. The suggested flow print helper can't reliably print
>> these two components ("set vxlan" and the flow rule itself) as they
>> belong to different testpmd command strings.
>>
> Again, I agree with you but like my above answer, do we want a partial solution 
> in DPDK?

My answer is YES.

>> As you might see, completeness of the solution wouldn't necessarily be
>> reachable, even if full item/action coverage was provided.
>>
>> As for the item/action coverage itself, it's rather controversial. On
>> the one hand, yes, we should probably try to cover more items and
>> actions in the suggested patch, to the extent allowed by our current
>> priorities. But on the other hand, the existing coverage might not be
>> that poor: it's fairly elaborate and at least allows to print the most
>> common flow rules.
>>
> That is my main issue you are going to push something that is good for you
> and maybe some other cases, but it can't be used by all application, even with
> the most basic commands like encap.

Isn't it fair: if one wants to use something, be ready to help
and extend it. No pain, no gain :) Jokes aside - we're ready to
support "the most basic commands", just list it, but do not say
everything is basic. "everything" will delay the feature and
simply unfair to demand (IMHO).

IMHO, the feature would make flow API more friendly and easier
to debug, report bugs etc.

>> Yes, macros and some other cunning ways to cover more flow specifics
>> might come in handy, but, at the same time, can be rather error prone.
>> Sometimes it's more robust to just write the code out in full.
>>
> I'm always in favor of easy of extra complex but too hard is also not good.
> 
> Thanks,
> Ori
>> Thank you.
>>
>> On 30/05/2021 10:27, Ori Kam wrote:
>>> Hi Ivan,
>>>
>>> First nice idea and thanks for the picking up the ball.
>>>
>>> Before a detail review,
>>> The main thing I'm concerned about is that this print will be partially
>> supported,
>>> I know that you covered this issue by printing unknown for unsupported
>> item/actions,
>>> but this will mean that it is enough that one item/action is not supported
>> and already the
>>> flow can't be used in testpmd.
>>> To get full support it means that the developer needs to add such print
>> with each new
>>> item/action. I agree it is possible, but it has high overhead for each feature.
>>>
>>> Maybe we should somehow create a macros for the prints or other easier
>> to support ways.
>>>
>>> For example, just printing the ipv4 has 7 function calls inside of it each one
>> with error checking,
>>> and I'm not counting the dedicated functions.
>>>
>>>
>>>
>>> Best,
>>> Ori
>>>
>>>
>>>> -----Original Message-----
>>>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>>>> Sent: Thursday, May 27, 2021 11:25 AM
>>>> To: dev@dpdk.org
>>>> Cc: NBU-Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh
>> Yigit
>>>> <ferruh.yigit@intel.com>; Andrew Rybchenko
>>>> <andrew.rybchenko@oktetlabs.ru>; Ori Kam <orika@nvidia.com>; Ray
>>>> Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>
>>>> Subject: [RFC PATCH] ethdev: add support for testpmd-compliant flow
>> rule
>>>> dumping
>>>>
>>>> DPDK applications (for example, OvS) or tests which use RTE flow API
>> need to
>>>> log created or rejected flow rules to help to recognise what goes right or
>>>> wrong. From this standpoint, testpmd-compliant format is nice for the
>>>> purpose because it allows to copy-paste the flow rules and debug using
>>>> testpmd.
>>>>
>>>> Recognisable pattern items:
>>>> VOID, VF, PF, PHY_PORT, PORT_ID, ETH, VLAN, IPV4, IPV6, UDP, TCP,
>> VXLAN,
>>>> NVGRE, GENEVE, MARK, PPPOES, PPPOED.
>>>>
>>>> Recognisable actions:
>>>> VOID, JUMP, MARK, FLAG, QUEUE, DROP, COUNT, RSS, PF, VF,
>> PHY_PORT,
>>>> PORT_ID, OF_POP_VLAN, OF_PUSH_VLAN, OF_SET_VLAN_VID,
>>>> OF_SET_VLAN_PCP, VXLAN_ENCAP, VXLAN_DECAP.
>>>>
>>>> Recognisable RSS types (action RSS):
>>>> IPV4, FRAG_IPV4, NONFRAG_IPV4_TCP, NONFRAG_IPV4_UDP,
>>>> NONFRAG_IPV4_OTHER, IPV6, FRAG_IPV6, NONFRAG_IPV6_TCP,
>>>> NONFRAG_IPV6_UDP, NONFRAG_IPV6_OTHER, IPV6_EX, IPV6_TCP_EX,
>>>> IPV6_UDP_EX, L3_SRC_ONLY, L3_DST_ONLY, L4_SRC_ONLY,
>> L4_DST_ONLY.
>>>>
>>>> Unrecognised parts of the flow specification are represented by tokens
>>>> "{unknown}" and "{unknown bits}". Interested parties are welcome to
>>>> extend this tool to recognise more items and actions.
>>>>
>>>> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>