RE: [RFC] ethdev: fast path async flow API

[Dariusz Sosnowski <dsosnowski@nvidia.com>]

Hi Stephen,

> > +/**
> > + * @internal
> > + *
> > + * Fast path async flow functions and related data are held in a flat array,
> one entry per ethdev.
> > + * It is assumed that each entry is read-only and cache aligned.
> > + */
> > +struct rte_flow_fp_ops {
> > +     void *ctx;
> > +     rte_flow_async_create_t async_create;
> > +     rte_flow_async_create_by_index_t async_create_by_index;
> > +     rte_flow_async_actions_update_t async_actions_update;
> > +     rte_flow_async_destroy_t async_destroy;
> > +     rte_flow_push_t push;
> > +     rte_flow_pull_t pull;
> > +     rte_flow_async_action_handle_create_t async_action_handle_create;
> > +     rte_flow_async_action_handle_destroy_t async_action_handle_destroy;
> > +     rte_flow_async_action_handle_update_t async_action_handle_update;
> > +     rte_flow_async_action_handle_query_t async_action_handle_query;
> > +     rte_flow_async_action_handle_query_update_t
> async_action_handle_query_update;
> > +     rte_flow_async_action_list_handle_create_t
> async_action_list_handle_create;
> > +     rte_flow_async_action_list_handle_destroy_t
> async_action_list_handle_destroy;
> > +     rte_flow_async_action_list_handle_query_update_t
> async_action_list_handle_query_update;
> > +} __rte_cache_aligned;
> 
> If you go ahead with this then all usage should be const pointer.
> Still think it is bad idea and creates even more technical debt.
Could you please elaborate a bit more on why do you think it is a bad idea and why it creates technical debt? 

> > **Future considerations**
> >
> > A case can be made about converting some of the existing stable API 
> > functions to fast path versions in future LTS releases.
> > I don't have any hard data on how such changes would affect 
> > performance of these APIs, but I assume that the improvement would be noticeable.
> 
> The problem is that inline functions create future ABI problems.
Agreed, this is why such a change can only be introduced when a new major ABI version is released.
However, even though inlining could reduce function call overhead, I'm not sure if such a change is needed for synchronous flow API.
I mentioned it here as a thing to consider.

> Usually, there are other ways to get the same performance benefit.
> Often batching updates to hardware will do the trick.
This is true, but we still have some library-level overhead.
To elaborate more, the current state of flow API is as follows:
- With sync flow API:
  - Applications cannot batch flow operations.
- With async flow APIs:
  - Applications can enqueue multiple flow operations and PMDs can issue batches to HW.
  - But there is still one function call per enqueued flow operation.
    The overall API overhead in datapath may be nonnegligible if we consider that applications may enqueue a flow rule creation operation for every packet received in SW.
This proposal specifically aims at reducing API overhead for async flow API in a case mentioned above.

As a side note, we plan to push changes to mlx5 PMD in 24.03 which will reduce PMD overhead in such scenarios.
This proposal's goal is to reduce overhead of async flow API at library level.

Best regards,
Dariusz Sosnowski