12/03/2021 08:09, Andrew Rybchenko:

On 3/12/21 1:17 AM, Thomas Monjalon wrote:

quoted

Since rte_flow is the only API for filtering operations,
the legacy driver interface filter_ctrl was too much complicated
for the simple task of getting the struct rte_flow_ops.

The filter type RTE_ETH_FILTER_GENERIC and
the filter operarion RTE_ETH_FILTER_GET are removed.
The new driver callback flow_ops_get replaces filter_ctrl.

Signed-off-by: Thomas Monjalon <redacted>

A couple of minor notes below. Other than that:

Reviewed-by: Andrew Rybchenko <redacted>

[snip]

quoted

diff --git a/doc/guides/rel_notes/release_20_11.rst b/doc/guides/rel_notes/release_20_11.rst
index 7405a9864f..1260539a21 100644
--- a/doc/guides/rel_notes/release_20_11.rst
+++ b/doc/guides/rel_notes/release_20_11.rst

@@ -571,7 +571,7 @@ API Changes
   a TC is greater than 256.
 
 * ethdev: Removed the legacy filter API, including
-  ``rte_eth_dev_filter_supported()`` and ``rte_eth_dev_filter_ctrl()``.
+  ``rte_eth_dev_filter_supported()`` and ``rte_eth_dev_flow_ops_get()``.

Mass substitution is dangerous. I guess this one is not
intended.

quoted

 
 * ethdev: Removed the legacy L2 tunnel configuration API, including
   ``rte_eth_dev_l2_tunnel_eth_type_conf()`` and

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index cea5c8746d..f7deeac34b 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst

@@ -501,7 +501,7 @@ API Changes
 -----------
 
 * The deprecated flow director API is removed.
-  It was replaced by ``rte_eth_dev_filter_ctrl()``.
+  It was replaced by ``rte_eth_dev_flow_ops_get()``.

As well as this one.

Oops, good catch, sorry I did not double check this part.

[...]

quoted

-typedef int (*eth_filter_ctrl_t)(struct rte_eth_dev *dev,
-				 enum rte_filter_type filter_type,
-				 enum rte_filter_op filter_op,
-				 void *arg);
-/**< @internal Take operations to assigned filter type on an Ethernet device */
+struct rte_flow_ops;
+typedef int (*eth_flow_ops_get_t)(struct rte_eth_dev *dev,
+				  const struct rte_flow_ops **ops);
+/**< @internal Get flow operations */

I'm OK with the callback prototype. I think there is no point
to optimize it and make ops a return value. It is called in
just one place. Nothing to optimize.

OK

quoted

--- a/lib/librte_ethdev/rte_flow.c
+++ b/lib/librte_ethdev/rte_flow.c

@@ -255,12 +255,9 @@ rte_flow_ops_get(uint16_t port_id, struct rte_flow_error *error)
 
 	if (unlikely(!rte_eth_dev_is_valid_port(port_id)))
 		code = ENODEV;
-	else if (unlikely(!dev->dev_ops->filter_ctrl ||
-			  dev->dev_ops->filter_ctrl(dev,
-						    RTE_ETH_FILTER_GENERIC,
-						    RTE_ETH_FILTER_GET,
-						    &ops) ||
-			  !ops))
+	else if (unlikely(!dev->dev_ops->flow_ops_get ||
+			  dev->dev_ops->flow_ops_get(dev, &ops) ||

If the callback return error code, may be it should be used
and forwarded here?

quoted

+			  ops == NULL))
 		code = ENOSYS;

Yes you are right.
Strange that it was not already forwarded.