Hi Andrew, > -----Original Message----- > From: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > Subject: Re: [PATCH v5 02/10] ethdev: add flow item/action templates > > On 2/12/22 01:25, Alexander Kozyrev wrote: > > On Fri, Feb 11, 2022 6:27 Andrew Rybchenko <andrew.rybche...@oktetlabs.ru> > > wrote: > >> On 2/11/22 05:26, Alexander Kozyrev wrote: > >>> Treating every single flow rule as a completely independent and separate > >>> entity negatively impacts the flow rules insertion rate. Oftentimes in an > >>> application, many flow rules share a common structure (the same item mask > >>> and/or action list) so they can be grouped and classified together. > >>> This knowledge may be used as a source of optimization by a PMD/HW. > >>> > >>> The pattern template defines common matching fields (the item mask) > >>> without > >>> values. The actions template holds a list of action types that will be > >>> used > >>> together in the same rule. The specific values for items and actions will > >>> be given only during the rule creation. > >>> > >>> A table combines pattern and actions templates along with shared flow rule > >>> attributes (group ID, priority and traffic direction). This way a PMD/HW > >>> can prepare all the resources needed for efficient flow rules creation in > >>> the datapath. To avoid any hiccups due to memory reallocation, the maximum > >>> number of flow rules is defined at the table creation time. > >>> > >>> The flow rule creation is done by selecting a table, a pattern template > >>> and an actions template (which are bound to the table), and setting unique > >>> values for the items and actions. > >>> > >>> Signed-off-by: Alexander Kozyrev <akozy...@nvidia.com> > >>> Acked-by: Ori Kam <or...@nvidia.com> > > [snip] >
[Snip] > >>> + * > >>> + * The pattern template defines common matching fields without values. > >>> + * For example, matching on 5 tuple TCP flow, the template will be > >>> + * eth(null) + IPv4(source + dest) + TCP(s_port + d_port), > >>> + * while values for each rule will be set during the flow rule creation. > >>> + * The number and order of items in the template must be the same > >>> + * at the rule creation. > >>> + * > >>> + * @param port_id > >>> + * Port identifier of Ethernet device. > >>> + * @param[in] template_attr > >>> + * Pattern template attributes. > >>> + * @param[in] pattern > >>> + * Pattern specification (list terminated by the END pattern item). > >>> + * The spec member of an item is not used unless the end member is > >>> used. > >> > >> Interpretation of the pattern may depend on transfer vs non-transfer > >> rule to be used. It is essential information and we should provide it > >> when pattern template is created. > >> > >> The information is provided on table stage, but it is too late. > > > > Why is it too late? Application knows which template goes to which table. > > And the pattern is generic to accommodate anything, user just need to put it > > into the right table. > > Because it is more convenient to handle it when individual > template is processed. Otherwise error reporting will be > complicated since it could be just one template which is > wrong. > > Otherwise, I see no point to have driver callbacks > template creation API. I can do nothing here since > I have no enough context. What's the problem to add > the context? > The idea is that the same template can be used in different domains (ingress/egress and transfer) May be we can add on which domains this template is expected to be used. What do you think? > > > >> > >>> + * @param[out] error > >>> + * Perform verbose error reporting if not NULL. > >>> + * PMDs initialize this structure in case of error only. > >>> + * > >>> + * @return > >>> + * Handle on success, NULL otherwise and rte_errno is set. > >>> + */ > >>> +__rte_experimental > >>> +struct rte_flow_pattern_template * > >>> +rte_flow_pattern_template_create(uint16_t port_id, > >>> + const struct rte_flow_pattern_template_attr *template_attr, > >>> + const struct rte_flow_item pattern[], > >>> + struct rte_flow_error *error); > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice. > >>> + * > >>> + * Destroy pattern template. > >> > >> Destroy flow pattern template. > > > > Ok. > > > >>> + * > >>> + * This function may be called only when > >>> + * there are no more tables referencing this template. > >>> + * > >>> + * @param port_id > >>> + * Port identifier of Ethernet device. > >>> + * @param[in] pattern_template > >>> + * Handle of the template to be destroyed. > >>> + * @param[out] error > >>> + * Perform verbose error reporting if not NULL. > >>> + * PMDs initialize this structure in case of error only. > >>> + * > >>> + * @return > >>> + * 0 on success, a negative errno value otherwise and rte_errno is set. > >>> + */ > >>> +__rte_experimental > >>> +int > >>> +rte_flow_pattern_template_destroy(uint16_t port_id, > >>> + struct rte_flow_pattern_template *pattern_template, > >>> + struct rte_flow_error *error); > >>> + > >>> +/** > >>> + * Opaque type returned after successful creation of actions template. > >>> + * This handle can be used to manage the created actions template. > >>> + */ > >>> +struct rte_flow_actions_template; > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice. > >>> + * > >>> + * Flow actions template attributes. > >>> + */ > >>> +struct rte_flow_actions_template_attr; > >>> + > >>> +/** > >>> + * @warning > >>> + * @b EXPERIMENTAL: this API may change without prior notice. > >>> + * > >>> + * Create actions template. > >> > >> Create flow rule actions template. > > > > Yes, finally compensating for multiple no's. > > > >>> + * > >>> + * The actions template holds a list of action types without values. > >>> + * For example, the template to change TCP ports is TCP(s_port + d_port), > >>> + * while values for each rule will be set during the flow rule creation. > >>> + * The number and order of actions in the template must be the same > >>> + * at the rule creation. > >> > >> Again, it highly depends on transfer vs non-transfer. Moreover, > >> application definitely know it. So, it should say if the action > >> is intended for transfer or non-transfer flow rule. > > > > It is up to application to define which pattern it is going to use in > > different tables. > > Same as above. > Same comment as above, what do you think? > > > >>> + * > >>> + * @param port_id > >>> + * Port identifier of Ethernet device. > >>> + * @param[in] template_attr > >>> + * Template attributes. > >>> + * @param[in] actions > >>> + * Associated actions (list terminated by the END action). > >>> + * The spec member is only used if @p masks spec is non-zero. > >>> + * @param[in] masks > >>> + * List of actions that marks which of the action's member is constant. > >>> + * A mask has the same format as the corresponding action. > >>> + * If the action field in @p masks is not 0, > >>> + * the corresponding value in an action from @p actions will be the > >>> part > >>> + * of the template and used in all flow rules. > >>> + * The order of actions in @p masks is the same as in @p actions. > >>> + * In case of indirect actions present in @p actions, > >>> + * the actual action type should be present in @p mask. > >>> + * @param[out] error > >>> + * Perform verbose error reporting if not NULL. > >>> + * PMDs initialize this structure in case of error only. > >>> + * > >>> + * @return > >>> + * Handle on success, NULL otherwise and rte_errno is set. > >>> + */ > >>> +__rte_experimental > >>> +struct rte_flow_actions_template * > >>> +rte_flow_actions_template_create(uint16_t port_id, > >>> + const struct rte_flow_actions_template_attr *template_attr, > >>> + const struct rte_flow_action actions[], > >>> + const struct rte_flow_action masks[], > >>> + struct rte_flow_error *error); > > [snip] Best, Ori
