Re: [dpdk-dev] [PATCH] doc: flow rule removal on port stop

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

On 11/18/20 11:59 AM, Gregory Etelson wrote:
> Hello Andrew,
> 
>> On 11/17/20 10:18 PM, Gregory Etelson wrote:
>>> There is a discrepancy between RTE ETHDEV API and flow rules guide
>>> regarding flow rules maintenance after port stop.  RTE ETHDEV API in
>>> librte_ethdev.h declares that flow rules will not be stored in PMD
>>> after port stop:
>>>   >>>>> Quite start
>>>   Please note that some configuration is not stored between calls to
>>>   rte_eth_dev_stop()/rte_eth_dev_start(). The following configuration
>>>   will be retained:
>>>
>>>   - MTU
>>>   - flow control settings
>>>   - receive mode configuration (promiscuous mode, all-multicast mode,
>>>     hardware checksum mode, RSS/VMDQ settings etc.)
>>>   - VLAN filtering configuration
>>>   - default MAC address
>>>   - MAC addresses supplied to MAC address array
>>>   - flow director filtering mode (but not filtering rules)
>>>   - NIC queue statistics mappings
>>>   <<<< Quote end
>>>
>>> PMD cannot always correctly restore flow rules after port stop / port
>>> start because application may alter port configuration after port stop
>>> without PMD knowledge about undergoing changes.  Consider the
>>> following scenario:
>>> application configures 2 queues 0 and 1 and creates a flow rule with
>>> 'queue index 1' action. After that application stops the port and
>>> removes queue 1.
>>> Although PMD can implement flow rule shadow copy to be used for
>>> restore after port start, attempt to restore flow rule from shadow
>>> will fail in example above and PMD could not notify application about
>>> that failure.  As the result, flow rules map in HW will differ from
>>> what application expects.  In addition, flow rules shadow copy used
>>> for port start restore consumes considerable amount of system memory,
>>> especially in systems with millions of flow rules.
>>>
>>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>>> Acked-by: Ori Kam <orika@nvidia.com>
>>> ---
>>>   doc/guides/prog_guide/rte_flow.rst | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/doc/guides/prog_guide/rte_flow.rst
>>> b/doc/guides/prog_guide/rte_flow.rst
>>> index 944e8242d6..dfe5a40f8e 100644
>>> --- a/doc/guides/prog_guide/rte_flow.rst
>>> +++ b/doc/guides/prog_guide/rte_flow.rst
>>> @@ -3055,10 +3055,9 @@ Caveats
>>>     temporarily replacing the burst function pointers), an appropriate
>> error
>>>     code must be returned (``EBUSY``).
>>>
>>> -- PMDs, not applications, are responsible for maintaining flow rules
>>> +- Applications, not PMDs, are responsible for maintaining flow rules
>>>     configuration when stopping and restarting a port or performing
>>> other
>>> -  actions which may affect them. They can only be destroyed
>>> explicitly by
>>> -  applications.
>>> +  actions which may affect them.
>>>
>>>   For devices exposing multiple ports sharing global settings affected
>> by flow
>>>   rules:
>>>
>>
>> Re-reading it, it still looks vague. What happens on:
>>   - port stop without removal of flow rule before
>>   - port close without removal of flow rules before
>>   - port reset (which could be stop/start, e.g. to recover from error
>> condition)
> 
> PMD should remove all flows related to hardware resource that was invalidated.

Stop? Close? I agree and documentation should say so in a bit
clear way.