Hi Ilya,

Thanks for reviewing this. Please see below.

On Thu, 29 Jun 2023, Ilya Maximets wrote:

On 6/6/23 13:35, Ivan Malov wrote:
Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

AFAICT, not all drivers moved to a REPRESENTED_PORT action.
I don't see support in NFP driver, for example.

This is the full story:

1) On Oct 13, 2021, commit [1] introduced
   a deprecation note for action PORT_ID.

2) On Aug 13, 2022, series [2] was sent to provide replacement
   for action PORT_ID in drivers that still had been using it.
   The series was accepted on Aug 22. NFP was not among the
   drivers to address because it had not been using PORT_ID.

3) On Oct 21, 2022, commit [3] added support for action PORT_ID to NFP.

Based on this input, it appears that deprecation notice for
action PORT_ID might have been ignored by NFP developers
for some reason. They added support for this action
after series [2] had been applied, so developers
behind [2] could not have a chance to offer help.

[1] 5da44faa809a ("ethdev: deprecate hard-to-use or ambiguous items and 
actions")
[2] 
https://patchwork.dpdk.org/project/dpdk/list/?series=24302&archive=both&state=*
[3] 4d946034bf9c ("net/nfp: support basic flow actions")


Are we OK with dropping hardware offload support for NFP ?

I'm not saying above explanation justifies dropping such support,
but perhaps it pays to ask NFP team for their opinion on this.

Thank you.

Or are there plans to support this new action in that driver?

Simon, maybe you have some visibility on that?

Best regards, Ilya Maximets.


Signed-off-by: Ivan Malov <ivan.ma...@arknetworks.am>
---
 lib/netdev-offload-dpdk.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 14bc87771..ef0a266c5 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -735,14 +735,14 @@ dump_flow_action(struct ds *s, struct ds *s_extra,
         ds_put_cstr(s, "rss / ");
     } else if (actions->type == RTE_FLOW_ACTION_TYPE_COUNT) {
         ds_put_cstr(s, "count / ");
-    } else if (actions->type == RTE_FLOW_ACTION_TYPE_PORT_ID) {
-        const struct rte_flow_action_port_id *port_id = actions->conf;
+    } else if (actions->type == RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT) {
+        const struct rte_flow_action_ethdev *ethdev = actions->conf;
+
+        ds_put_cstr(s, "represented_port ");
+
+        if (ethdev)
+            ds_put_format(s, "ethdev_port_id %d ", ethdev->port_id);

-        ds_put_cstr(s, "port_id ");
-        if (port_id) {
-            ds_put_format(s, "original %d id %d ",
-                          port_id->original, port_id->id);
-        }
         ds_put_cstr(s, "/ ");
     } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
         ds_put_cstr(s, "drop / ");
@@ -1776,19 +1776,22 @@ add_count_action(struct flow_actions *actions)
 }

 static int
-add_port_id_action(struct flow_actions *actions,
-                   struct netdev *outdev)
+add_represented_port_action(struct flow_actions *actions,
+                            struct netdev *outdev)
 {
-    struct rte_flow_action_port_id *port_id;
+    struct rte_flow_action_ethdev *ethdev;
     int outdev_id;

     outdev_id = netdev_dpdk_get_port_id(outdev);
     if (outdev_id < 0) {
         return -1;
     }
-    port_id = xzalloc(sizeof *port_id);
-    port_id->id = outdev_id;
-    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_PORT_ID, port_id);
+
+    ethdev = xzalloc(sizeof *ethdev);
+    ethdev->port_id = outdev_id;
+
+    add_flow_action(actions, RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT, ethdev);
+
     return 0;
 }

@@ -1808,7 +1811,7 @@ add_output_action(struct netdev *netdev,
         return -1;
     }
     if (!netdev_flow_api_equals(netdev, outdev) ||
-        add_port_id_action(actions, outdev)) {
+        add_represented_port_action(actions, outdev)) {
         VLOG_DBG_RL(&rl, "%s: Output to port \'%s\' cannot be offloaded.",
                     netdev_get_name(netdev), netdev_get_name(outdev));
         ret = -1;


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to