04/06/2020 13:12, Andrey Vesnovaty:

Thomas Monjalon [off-list ref] wrote:

quoted

20/05/2020 11:18, Andrey Vesnovaty:
We had "create", "destroy", "query", but no "modify" capability.
The new API is adding 2 things in my opinion:
        - shared action object
        - "modify" capability (is "update" a better wording?)

Naming is one of the most challenging parts of this RFC.
Some similarity I have found in existing code is
rte_mtr_policer_actions_update()
Is there any existing code having update/modify semantics?

Except one callback in librte_fib, no DPDK API has "modify" in its name.
You can find the word "update" in the API of multiple DPDK libs.
I would like having the opinion of a native english speaker here.

quoted

About the wording, do we need "ctx"?
I feel rte_flow_action is a good enough prefix for this API,
and should be documented as a shared action object.
I think the word "object" is more meaningful than "context".
Am I missing something?

CTX comes for the fact that each flow_rule doesn't have an ownership for
the given action but operates inside some context (shared action context
actually).
As mentioned above, naming is one of the most challenging parts of this
RFC.

I think we can replace "context" with "object" in the explanations.
The functions could be named without "ctx":
	- rte_flow_action_create
	- rte_flow_action_destroy
	- rte_flow_action_update
	- rte_flow_action_query

quoted

quoted

+     /**
+      * Describes action context.
+      *
+      * Enables multiple rules reference the same action by id/ctx.
+      *
+      * No action specific struct here (void*) since it can be any
+      * action type.
+      */
+     RTE_FLOW_ACTION_TYPE_CTX,

Why do we need a new action type?

Because it's not an action itself but a reference/handle to it.

Sorry I don't understand when RTE_FLOW_ACTION_TYPE_CTX has to be used.
There is no mention of RTE_FLOW_ACTION_TYPE_CTX in the explanations.