On Tuesday, February 8, 2022 10:24 Ivan Malov <ivan.ma...@oktetlabs.ru> wrote: > On Tue, 8 Feb 2022, Alexander Kozyrev wrote: > >> > >>> + > >>> +Enqueue creation operation > >>> +~~~~~~~~~~~~~~~~~~~~~~~~~~ > >>> + > >>> +Enqueueing a flow rule creation operation is similar to simple creation. > >> > >> If it is enqueue operation, why not call it ad rte_flow_q_flow_enqueue() > >> > >>> + > >>> +.. code-block:: c > >>> + > >>> + struct rte_flow * > >>> + rte_flow_q_flow_create(uint16_t port_id, > >>> + uint32_t queue_id, > >>> + const struct rte_flow_q_ops_attr > >>> *q_ops_attr, > >>> + struct rte_flow_table *table, > >>> + const struct rte_flow_item pattern[], > >>> + uint8_t pattern_template_index, > >>> + const struct rte_flow_action actions[], > >> > >> If I understand correctly, table is the pre-configured object that has > >> N number of patterns and N number of actions. > >> Why giving items[] and actions[] again? > > > > Table only contains templates for pattern and actions. > > Then why not reflect it in the argument name? Perhaps, "template_table"? > Or even in the struct name: "struct rte_flow_template_table". > Chances are that readers will misread "rte_flow_table" > as "flow entry table" in the OpenFlow sense. > > We still need to provide the values for those templates when we create a > flow. > > Thus we specify patterns and action here. > > All of that is clear in terms of this review cycle, but please > consider improving the argument names to help future readers.
Agree, it is a good idea to rename it to template_table, thanks. > > > >>> + uint8_t actions_template_index, > >>> + struct rte_flow_error *error); > >>> + > >>> +A valid handle in case of success is returned. It must be destroyed later > >>> +by calling ``rte_flow_q_flow_destroy()`` even if the rule is rejected by > >> HW. > >>> + > >>> +Enqueue destruction operation > >>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >> > >> Queue destruction operation. > > > > We are not destroying queue, we are enqueuing the flow destruction > operation. 7
