04/06/2020 13:12, Andrey Vesnovaty: > Thomas Monjalon <tho...@monjalon.net> wrote: > > 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. > > 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 > > > + /** > > > + * 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.