Re: [dpdk-dev] [RFC] add flow action context API

[Jerin Jacob <jerinjacobk@gmail.com>]

I would suggest adding rte_flow driver implementers if there is a
change in rte_flow_ops in RFC so that
your patch will get enough. I added the maintainers of rte_flow PMD[1]
implementers in Cc.


>>
>> Would the following additional API suffice the motivation?
>>
>> rte_flow_modify_action(struct rte_flow * flow,  const struct
>> rte_flow_action actions[])
>
>
> This API limits the scope to single flow which isn't the goal for the proposed change.

Yes. But we need to find the balance between HW features(driver
interface) and public API?
Is Mellanox HW has the support for native shared HW ACTION context?
if so, it makes sense to have such a fat public API as you suggested.
If not, it is a matter of application to iterate over needed flows to
modify action through rte_flow_modify_action().

Assume Mellanox HW has native HW ACTION context and the majority of
the other HW[1] does not
have, then IMO, We should have common code, to handle in this complex
state machine
implementation of action_ctx_create, action_ctx_destroy,
rte_flow_action_ctx_modify, action_ctx_query
in case PMD does not support it. (If PMD/HW supports it then it can
use native implementation)

Reason:
1) I think, All the HW will have the the option to update the ACTION
for the given flow.
octeontx2 has it. If not, let's discuss what is typical HW abstraction
ACTION only update.
2) This case can be implemented if PMD just has flow_modify_action() support.
Multiple flows will be matter will be iterating over all registered flow.
3) Avoid code duplication on all of the below PMDs[1]



[1]
drivers/net/hinic/hinic_pmd_flow.c:const struct rte_flow_ops hinic_flow_ops = {
drivers/net/ipn3ke/ipn3ke_flow.h:extern const struct rte_flow_ops
ipn3ke_flow_ops;
drivers/net/i40e/i40e_flow.c:const struct rte_flow_ops i40e_flow_ops = {
drivers/net/qede/qede_filter.c:const struct rte_flow_ops qede_flow_ops = {
drivers/net/octeontx2/otx2_flow.h:extern const struct rte_flow_ops
otx2_flow_ops;
drivers/net/ice/ice_generic_flow.h:extern const struct rte_flow_ops
ice_flow_ops;
drivers/net/tap/tap_flow.c:static const struct rte_flow_ops tap_flow_ops = {
drivers/net/dpaa2/dpaa2_ethdev.h:extern const struct rte_flow_ops
dpaa2_flow_ops;
drivers/net/e1000/e1000_ethdev.h:extern const struct rte_flow_ops igb_flow_ops;
drivers/net/enic/enic_flow.c:const struct rte_flow_ops enic_flow_ops = {
drivers/net/bonding/rte_eth_bond_flow.c:const struct rte_flow_ops
bond_flow_ops = {
drivers/net/mlx5/mlx5_flow.c:static const struct rte_flow_ops mlx5_flow_ops = {
drivers/net/igc/igc_flow.h:extern const struct rte_flow_ops igc_flow_ops;
drivers/net/cxgbe/cxgbe_flow.c:static const struct rte_flow_ops
cxgbe_flow_ops = {
drivers/net/failsafe/failsafe_private.h:extern const struct
rte_flow_ops fs_flow_ops;
drivers/net/mvpp2/mrvl_flow.c:const struct rte_flow_ops mrvl_flow_ops = {
drivers/net/iavf/iavf_generic_flow.c:const struct rte_flow_ops iavf_flow_ops = {
drivers/net/hns3/hns3_flow.c:static const struct rte_flow_ops hns3_flow_ops = {
drivers/net/bnxt/bnxt_flow.c:const struct rte_flow_ops bnxt_flow_ops = {
drivers/net/mlx4/mlx4_flow.c:static const struct rte_flow_ops mlx4_flow_ops = {
drivers/net/sfc/sfc_flow.c:const struct rte_flow_ops sfc_flow_ops = {
drivers/net/softnic/rte_eth_softnic_flow.c:const struct rte_flow_ops
pmd_flow_ops = {
drivers/net/ixgbe/ixgbe_ethdev.h:extern const struct rte_flow_ops
ixgbe_flow_ops;