On 10/15/21 12:04 PM, Dmitry Kozlyuk wrote: >> [...] >>>>> From: Dmitry Kozlyuk <dkozl...@nvidia.com> >>>>> >>>>> rte_flow_action_handle_create() did not mention what happens with an >>>>> indirect action when a device is stopped, possibly reconfigured, and >>>>> started again. It is natural for some indirect actions to be >>>>> persistent, like counters and meters; keeping others just saves >>>>> application time and complexity. However, not all PMDs can support it. >>>>> It is proposed to add a device capability to indicate if indirect >>>>> actions are kept across the above sequence or implicitly destroyed. >>>>> >>>>> In the future, indirect actions may not be the only type of objects >>>>> shared between flow rules. The capability bit intends to cover all >>>>> possible types of such objects, hence its name. >>>>> >>>>> It may happen that in the future a PMD acquires support for a type >>>>> of shared objects that it cannot keep across a restart. It is >>>>> undesirable to stop advertising the capability so that applications >>>>> that don't use objects of the problematic type can still take >> advantage of it. >>>>> This is why PMDs are allowed to keep only a subset of shared objects >>>>> provided that the vendor mandatorily documents it. >>>>> >>>>> If the device is being reconfigured in a way that is incompatible >>>>> with an existing shared objects, PMD is required to report an error. >>>>> This is mandatory, because flow API does not supply users with >>>>> capabilities, so this is the only way for a user to learn that >>>>> configuration is invalid. For example, if queue count changes and >>>>> RSS indirect action specifies queues that are going away, the user >>>>> must update the action before removing the queues or remove the >>>>> action and all flow rules that were using it. >>>>> >>>>> Signed-off-by: Dmitry Kozlyuk <dkozl...@nvidia.com> >>>>> --- >>>>> [...] >>>> >>>> Current pain point is that capability bits may be insufficient and a >>>> programmatic way is desired to check which types of objects can be >>>> kept across restart, instead of documenting the limitations. >>>> >>>> I support one of previous Ori's suggestions and want to clarify it [1]: >>>> >>>> Ori: "Another way is to assume that if the action was created before >>>> port start it will be kept after port stop." >>>> Andrew: "It does not sound like a solution. May be I simply don't >>>> know target usecase." >>>> >>>> What Ori suggests (offline discussion summary): Suppose an >>>> application wants to check whether a shared object (indirect action) >>>> or a flow rule of a particular kind. It calls >>>> rte_flow_action_handle_create() or >>>> rte_flow_create() before rte_eth_dev_start(). If it succeeds, 1) it >>>> means objects of this type can be kept across restart, 2) it's a >>>> normal object created that will work after the port is started. This >>>> is logical, because if the PMD can keep some kind of objects when the >>>> port is stopped, it is likely to be able to create them when the port >>>> is not started. It is subject to discussion if "object kind" means >>>> only "type" or "type + transfer bit" combination; for mlx5 PMD it >>>> doesn't matter. One minor drawback is that applications can only do >>>> the test when the port is stopped, but it seems likely that the test >>>> really needs to be done at startup anyway. >>>> >>>> If this is acceptable: >>>> 1. Capability bits are not needed anymore. >>>> 2. ethdev patches can be accepted in RC1, present behavior is >>>> undefined anyway. >>>> 3. PMD patches will need update that can be done by RC2. >>> >>> Andrew, what do you think? >>> If you agree, do we need to include transfer bit into "kind"? >>> I'd like to conclude before RC1 and can update the docs quickly. >>> >>> I've seen the proposition to advertise capability to create flow rules >>> before device start as a flag. >>> I don't think it conflicts with Ori's suggestion because the flag >>> doesn't imply that _any_ rule can be created, neither does it say >>> about indirect actions. >>> On the other hand, if PMD can create a flow object (rule, etc.) when >>> the device is not started, it is logical to assume that after the >>> device is stopped it can move existing flow objects to the same state >>> as when the device was not started, then restore when it is started >>> again. >> >> Dmitry, thanks for the explanations. Ori's idea makes sense to me now. The >> problem is to document it properly. We must define rules to check it. >> Which bits in the check request matter and how application should make a >> choice of rule to try. > > This is a generalization of the last question about the transfer bit. > I call the bits that matter a "kind". As I see it: > > rule kind = seq. of item types + seq. of action types > indirect action kind = action type > > As Ori mentioned, for mlx5 PMD transfer bit doesn't affect object persistence. > If you or other PMD maintainers think it may be relevant, no problem, > because PMDs like mlx5 will just ignore it when checking. Then it will be: > > rule kind = seq. of item types + seq. of action types + attr. transfer bit > indirect action kind = action type + indirect action conf. transfer bit > > Including the transfer bit seems to be a safe choice from DPDK point of view, > but obviously it can force applications to do up to twice as many checks. > > The application needs to construct a valid flow configuration > that is (1) valid and (2) has the kind the application is interested in. > It is worth noting that these checks are not about resource consumption, > i.e. it is sufficient to test an indirect RSS with one queue > to be confident that indirect RSS with any number of queues are preserved. > >> Which status code should be returned by the PMD to >> clearly say that addition in started state is not supported and, >> therefore, preserving across restart is not supported. > > I suggest a new DPDK-specific value in rte_errno.h. > >> Must the device be configured before an attempt to check it? > > Yes, because flow objects created by these checks are as good as any others > and AFAIK no PMD supports rte_flow calls before configuration is done. > >> Should transfer and non-transfer rules/indirect actions be checked >> separately? > > See above. >
Please, try to put it into the patch for documentation and I'll review the result. All my above questions should be answered in the documentation.
