Re: [dpdk-dev] [PATCH v3 1/6] ethdev: add capability to keep flow rules on restart

[Dmitry Kozlyuk <dkozlyuk@oss.nvidia.com>]

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: 19 октября 2021 г. 19:39
> To: Dmitry Kozlyuk <dkozlyuk@nvidia.com>; dev@dpdk.org; Qi Zhang
> <qi.z.zhang@intel.com>
> Cc: Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas Monjalon
> <thomas@monjalon.net>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Subject: Re: [PATCH v3 1/6] ethdev: add capability to keep flow rules on
> restart
> 
> External email: Use caution opening links or attachments
> 
> 
> On 10/19/2021 1:37 PM, Dmitry Kozlyuk wrote:
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> b/doc/guides/prog_guide/rte_flow.rst
> > index 2b42d5ec8c..ff67b211e3 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -87,6 +87,31 @@ To avoid resource leaks on the PMD side, handles must
> be explicitly
> >   destroyed by the application before releasing associated resources
> such as
> >   queues and ports.
> >
> > +If ``RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP`` is not advertised,
> > +rules cannot be created until the device is started for the first time
> > +and cannot be kept when the device is stopped.
> 
> So flag means two things:
> 1) rules cannot be created until the device is started for the first time
> 2) rules cannot be kept when the device is stopped
> 
> Can't be a case one is true but other is not? I was thinking flag is
> only for (2).

It theoretically can, but it doesn't seem feasible
to separate these capabilities:

a) Suppose a PMD can create rules before the device is started.
They are in some special state when they are not applied to the traffic.
When the device is started, these rules begin being applied.
When the device is stopped, what would make the PMD unable to move
the rules back to the detached state they were before the start?
And then attach them back?

b) Suppose a PMD can keep the rules between stop and start.
It must be able to move them to the detached stated described above
on the device stop and attach them back when it is started again.
What would prevent the PMD to create rules in a detached state
before the device is started for the first time?

That's what I had in mind before,
and now it is just stated explicitly per Andrew's suggestion:
https://inbox.dpdk.org/dev/5ec7101f-169e-cbd0-87bb-810b7476c7d0@oktetlabs.ru

> > +However, PMD also does not flush them automatically on stop,
> > +so the application must call ``rte_flow_flush()`` or
> ``rte_flow_destroy()``
> > +before stopping the device to ensure no rules remain.
> > +
> > +If ``RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP`` is advertised, this means
> > +the PMD can keep at least some rules across the device stop and start.
> > +However, ``rte_eth_dev_configure()`` may fail if any rules remain,
> > +so the application must flush them before attempting a reconfiguration.
> 
> If there are any remaining rules, should we fail the
> ``rte_eth_dev_configure()``,
> or is it allowed PMD to flush the rules itself?
>
> As far as I know some Intel PMDs flush remaining rules in configure itself
> without failing, @Qi can correct me if I am wrong.

Implicit flush is non-orthogonal API,
which only makes sense if it gives some performance benefit.

> > +Keeping may be unsupported for some types of rule items and actions,
> > +as well as depending on the value of flow attributes transfer bit.
> > +A combination of an item or action type and a value of the transfer bit
> > +is called a rule feature.
> > +To test if rules with a particular feature are kept, the application
> must try
> > +to create a valid rule using this feature when the device is stopped
> > +(after it has been configured or started previously).
> > +If it fails with an error of type ``RTE_FLOW_ERROR_TYPE_STATE``,
> > +rules using this feature are flushed when the device is stopped.
> > +If it suceeds, such rules will be kept when the device is stopped,
> > +provided they do not use other features that are not supported.
> > +Rules that are created when the device is stopped, including the rules
> > +created for the test, will be kept after the device is started.
> > +
> 
> I understand the intention, but I don't know if this is true for all
> devices.
> Can't there be a case that driver can't create rule when it is stopped,
> but it can keep the rules after stop. Or other-way around, driver can
> create rule when it is stopped, but can't keep rule after stop.

Isn't it the same consideration as the first comment?
If so, it's about the ability to have a rule in a detached state
given its features.

> I am feeling we are missing comments from different vendors if this logic
> works for them.