[ovs-dev] [PATCH v9 1/1] netdev-offload-dpdk: Replace action PORT_ID with REPRESENTED_PORT.

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 v8: split from 
https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/406049.html
 v9: address warnings by 0-robot regarding the summary line

 lib/netdev-offload-dpdk.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 992627fa2..623005b1c 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -735,14 +735,15 @@ 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, "port_id ");
-if (port_id) {
-ds_put_format(s, "original %d id %d ",
-  port_id->original, port_id->id);
+ds_put_cstr(s, "represented_port ");
+
+if (ethdev) {
+ds_put_format(s, "ethdev_port_id %d ", ethdev->port_id);
 }
+
 ds_put_cstr(s, "/ ");
 } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
 ds_put_cstr(s, "drop / ");
@@ -1776,19 +1777,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 +1812,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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.20.1

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

Re: [ovs-dev] [PATCH v2] dpdk: Update to use v23.11.

[Ivan Malov via dev]

Hi Ilya,

On Tue, 16 Jan 2024, Ilya Maximets wrote:


On 1/15/24 17:13, Kevin Traynor wrote:

On 15/01/2024 14:28, David Marchand wrote:

This commit adds support for DPDK v23.11.
It updates the CI script and documentation and includes the following
changes coming from the dpdk-latest branch:

- sparse: Add some compiler intrinsics for DPDK build.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=371129=*

- ci: Cache DPDK installed libraries only.
- ci: Reduce optional libraries in DPDK.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=383367=*

- system-dpdk: Ignore net/ice error log about QinQ offloading.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=385259=*

There is a known issue with i40e VF devices where OVS main thread may
block when adding such devices as dpif-netdev dpdk ports.

Signed-off-by: David Marchand 
---


Acked-by: Kevin Traynor 



I removed the mentioning of the internal 'dpif-netdev' name from
the NEWS file and applied the change.

Michael, could you, please, update the Intel CI to use 23.11 ?

Ivan, please, re-post (with a version bump) your rte_flow patch,
so we can accept it.


Done. See 
https://patchwork.ozlabs.org/project/openvswitch/patch/20240116004848.7957-1-ivan.ma...@arknetworks.am/
 .

Thank you.



Thanks!
Best regards, Ilya Maximets.


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

[ovs-dev] [PATCH v8 1/1] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 v8: split from 
https://mail.openvswitch.org/pipermail/ovs-dev/2023-June/406049.html

 lib/netdev-offload-dpdk.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 992627fa2..623005b1c 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -735,14 +735,15 @@ 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, "port_id ");
-if (port_id) {
-ds_put_format(s, "original %d id %d ",
-  port_id->original, port_id->id);
+ds_put_cstr(s, "represented_port ");
+
+if (ethdev) {
+ds_put_format(s, "ethdev_port_id %d ", ethdev->port_id);
 }
+
 ds_put_cstr(s, "/ ");
 } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
 ds_put_cstr(s, "drop / ");
@@ -1776,19 +1777,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 +1812,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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.20.1

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

Re: [ovs-dev] [PATCH] dpdk: Update to use v23.11.

[Ivan Malov via dev]

Ilya, David,

I appreciate your work.

With regard to the deferred change waiting to be revisited,
do you want me to take some action on it? Rework maybe?

Thank you.

On Sat, 16 Dec 2023, Ilya Maximets wrote:


On 12/13/23 14:06, David Marchand wrote:

This commit adds support for DPDK v23.11.
It updates the CI script and documentation and includes the following
changes coming from the dpdk-latest branch:

- sparse: Add some compiler intrinsics for DPDK build.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=371129=*

- ci: Cache DPDK installed libraries only.
- ci: Reduce optional libraries in DPDK.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=383367=*

- system-dpdk: Ignore net/ice error log about QinQ offloading.
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=385259=*

Signed-off-by: David Marchand 
---


Thanks, David!  I didn't test this much, but it looks
correct to me:

Acked-by: Ilya Maximets 


BTW, Just to not forget, there is one deferred change that
is waiting for 23.11 to be revisited:
 
https://patchwork.ozlabs.org/project/openvswitch/patch/20230630024630.6464-3-ivan.ma...@arknetworks.am/

Best regards, Ilya Maximets.


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

Re: [ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

Hi Ilya,

A quick follow-up from me: I made a new version of this patch [1].
[1] 
https://patchwork.ozlabs.org/project/openvswitch/patch/20230716115720.6789-1-ivan.ma...@arknetworks.am/

Thank you.

On Fri, 14 Jul 2023, Ilya Maximets wrote:


On 6/30/23 04:46, Ivan Malov wrote:

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 


Hi, Ivan.  Thanks for the patch!
I suppose, it can be considered as a bug fix.  Could you add
a Fixes tag to the commit message?


---
 lib/netdev-dpdk.c | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {

 /* Array of vhost rxq states, see vring_state_changed. */
 bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
 );

 PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }

+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */


This doesn't seem to be correct.  This flag is only needed for
tunnel offload.  Should also be under experimental api ifdef?


+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);


For all the logs above, use the following format instead:

   VLOG_*("%s: ...", netdev_get_name(>up));

For the last two, you may use:

   VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
"%s: Cannot negotiate Rx metadata: %s",
netdev_get_name(>up), rte_strerror(-ret));


+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;

+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);


It's a bit strange that we request metadata regardless of hw-offload
being enabled.  Should this be under netdev_is_flow_api_enabled() ?


+
 rte_eth_dev_info_get(dev->port_id, );

 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;

+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;

 ovs_list_push_back(_list, >list_node);




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

[ovs-dev] [PATCH v2 1/1] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Ivan Malov 
---
 v2: add missing experimental api ifdef

 lib/netdev-dpdk.c | 56 +++
 1 file changed, 56 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index aa87ee546..ea4cc6977 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -512,6 +512,9 @@ struct netdev_dpdk {
 
 /* Array of vhost rxq states, see vring_state_changed. */
 bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
 );
 
 PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1220,6 +1223,45 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+
+#ifdef ALLOW_EXPERIMENTAL_API
+/* For the tunnel offload  */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+#endif /* ALLOW_EXPERIMENTAL_API */
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("%s: The NIC will not provide per-packet USER_MARK",
+ netdev_get_name(>up));
+}
+#ifdef ALLOW_EXPERIMENTAL_API
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("%s: The NIC will not provide per-packet TUNNEL_ID",
+ netdev_get_name(>up));
+}
+#endif /* ALLOW_EXPERIMENTAL_API */
+} else {
+VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
+ "%s: Cannot negotiate Rx metadata: %s",
+ netdev_get_name(>up), rte_strerror(-ret));
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1233,6 +1275,18 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+if (netdev_is_flow_api_enabled()) {
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+}
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1421,6 +1475,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
 
+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
 
 ovs_list_push_back(_list, >list_node);
-- 
2.17.1

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

[ovs-dev] [PATCH 1/1] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Fixes: e8a2b5bf92bb ("netdev-dpdk: implement flow offload with rte flow")
Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 54 +++
 1 file changed, 54 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index aa87ee546..3d39a3afd 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -512,6 +512,9 @@ struct netdev_dpdk {
 
 /* Array of vhost rxq states, see vring_state_changed. */
 bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
 );
 
 PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1220,6 +1223,43 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+
+#ifdef ALLOW_EXPERIMENTAL_API
+/* For the tunnel offload  */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+#endif /* ALLOW_EXPERIMENTAL_API */
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("%s: The NIC will not provide per-packet USER_MARK",
+ netdev_get_name(>up));
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("%s: The NIC will not provide per-packet TUNNEL_ID",
+ netdev_get_name(>up));
+}
+} else {
+VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
+ "%s: Cannot negotiate Rx metadata: %s",
+ netdev_get_name(>up), rte_strerror(-ret));
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1233,6 +1273,18 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+if (netdev_is_flow_api_enabled()) {
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+}
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1421,6 +1473,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
 
+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
 
 ovs_list_push_back(_list, >list_node);
-- 
2.17.1

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

Re: [ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

Hi Ilya,

Thanks for reviewing this. PSB.

On Fri, 14 Jul 2023, Ilya Maximets wrote:


On 6/30/23 04:46, Ivan Malov wrote:

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 


Hi, Ivan.  Thanks for the patch!
I suppose, it can be considered as a bug fix.  Could you add
a Fixes tag to the commit message?


---
 lib/netdev-dpdk.c | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {

 /* Array of vhost rxq states, see vring_state_changed. */
 bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
 );

 PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }

+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */


This doesn't seem to be correct.  This flag is only needed for
tunnel offload.  Should also be under experimental api ifdef?


My guess, it's comment which is incorrect. I'm going to fix it, yes.
But, regarding the experimental api ifdef, according to review [1]
from Eli, it might be unneeded. Furthermore, the dpdk api being
invoked is no longer experimental. What do you think?

[1] 
https://patchwork.ozlabs.org/project/openvswitch/patch/20220720121823.2497727-4-ivan.ma...@oktetlabs.ru/#2935847




+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);


For all the logs above, use the following format instead:

   VLOG_*("%s: ...", netdev_get_name(>up));

For the last two, you may use:

   VLOG(ret == -ENOTSUP ? VLL_DBG : VLL_WARN,
"%s: Cannot negotiate Rx metadata: %s",
netdev_get_name(>up), rte_strerror(-ret));


+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;

+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);


It's a bit strange that we request metadata regardless of hw-offload
being enabled.  Should this be under netdev_is_flow_api_enabled() ?


+
 rte_eth_dev_info_get(dev->port_id, );

 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;

+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;

 ovs_list_push_back(_list, >list_node);




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

Re: [ovs-dev] [PATCH v6 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

Hi Ilya,

Thanks again for reviewing this. I attempted to fix review notes in
https://patchwork.ozlabs.org/project/openvswitch/list/?series=361784 .

Thank you.

On Thu, 29 Jun 2023, Ilya Maximets wrote:


On 6/29/23 15:58, Simon Horman wrote:

On Tue, Jun 06, 2023 at 03:35:32PM +0400, Ivan Malov via dev wrote:

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8cb1a7703..98765fe6e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -542,6 +542,9 @@ struct netdev_dpdk {
 int rte_xstats_names_size;
 int rte_xstats_ids_size;
 uint64_t *rte_xstats_ids;
+
+/* Ensures that Rx metadata delivery is configured only once */


This also needs a period at the end of a comment.


+bool rx_metadata_delivery_configured;


And this doesn't really belong in the 'stats' section of the structure.
Might be better fit in one of the previous sections.  The one with 'up',
for example.


 );
 };

@@ -1140,6 +1143,41 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }

+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured)
+return;


Hi Ivan,

IIRC, the OVS coding style calls for the use of { } for all conditional blocks.
So please update this to:

if (dev->rx_metadata_delivery_configured) {
return;
}

Likewise, a similar update is needed for patch 2/2.

Thanks!




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

Re: [ovs-dev] [PATCH v6 2/2] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

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=both=*
[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 
---
 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(, "%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

[ovs-dev] [PATCH v7 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 51 +++
 1 file changed, 51 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 63dac689e..d9d1b43f6 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -492,6 +492,9 @@ struct netdev_dpdk {
 
 /* Array of vhost rxq states, see vring_state_changed. */
 bool *vhost_rxq_enabled;
+
+/* Ensures that Rx metadata delivery is configured only once. */
+bool rx_metadata_delivery_configured;
 );
 
 PADDED_MEMBERS(CACHE_LINE_SIZE,
@@ -1187,6 +1190,42 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured) {
+return;
+}
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1200,6 +1239,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1382,6 +1431,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
 
+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
 
 ovs_list_push_back(_list, >list_node);
-- 
2.39.2

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

[ovs-dev] [PATCH v7 2/2] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 lib/netdev-offload-dpdk.c | 30 +-
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
index 14bc87771..40fdf03c1 100644
--- a/lib/netdev-offload-dpdk.c
+++ b/lib/netdev-offload-dpdk.c
@@ -735,14 +735,15 @@ 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, "port_id ");
-if (port_id) {
-ds_put_format(s, "original %d id %d ",
-  port_id->original, port_id->id);
+ds_put_cstr(s, "represented_port ");
+
+if (ethdev) {
+ds_put_format(s, "ethdev_port_id %d ", ethdev->port_id);
 }
+
 ds_put_cstr(s, "/ ");
 } else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
 ds_put_cstr(s, "drop / ");
@@ -1776,19 +1777,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 +1812,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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.39.2

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

[ovs-dev] [PATCH v7 0/2] DPDK: align flow offloads with 22.11

[Ivan Malov via dev]

Address two problems using two corresponding features in DPDK,
which have been around for a year and are now stable:

1) The need to make sure that metadata generated by flow
   rule execution be delivered from NIC to application;

2) Replacing PORT_ID action with REPRESENTED_PORT;
---
 v2: amendments to care about proxy detach and port ID logging
 v3: a minor adjustment in the cover letter subject line
 v4: edits to apply review notes and align with 22.11
 v5: drop the transfer proxy patch; fix CI in [1/2]
 v6: another attempt to fix re-configure in [1/2]
 v7: address review notes from Simon and Ilya

Ivan Malov (2):
  netdev-dpdk: negotiate delivery of per-packet Rx metadata
  netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

 lib/netdev-dpdk.c | 51 +++
 lib/netdev-offload-dpdk.c | 30 +--
 2 files changed, 68 insertions(+), 13 deletions(-)

-- 
2.39.2

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

Re: [ovs-dev] [PATCH v6 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

On Thu, 29 Jun 2023, Simon Horman wrote:


On Tue, Jun 06, 2023 at 03:35:32PM +0400, Ivan Malov via dev wrote:

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8cb1a7703..98765fe6e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -542,6 +542,9 @@ struct netdev_dpdk {
 int rte_xstats_names_size;
 int rte_xstats_ids_size;
 uint64_t *rte_xstats_ids;
+
+/* Ensures that Rx metadata delivery is configured only once */
+bool rx_metadata_delivery_configured;
 );
 };

@@ -1140,6 +1143,41 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }

+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured)
+return;


Hi Ivan,

IIRC, the OVS coding style calls for the use of { } for all conditional blocks.
So please update this to:

   if (dev->rx_metadata_delivery_configured) {
   return;
   }

Likewise, a similar update is needed for patch 2/2.

Thanks!


Hi Simon,

Very valuable information indeed. Now I understand
the true reason behind the check-bot warning.

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

[ovs-dev] [PATCH v6 2/2] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.17.1

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

[ovs-dev] [PATCH v6 0/2] DPDK: align flow offloads with 22.11

[Ivan Malov via dev]

Address two problems using two corresponding features in DPDK,
which have been around for a year and are now stable:

1) The need to make sure that metadata generated by flow
   rule execution be delivered from NIC to application;

2) Replacing PORT_ID action with REPRESENTED_PORT;
---
 v2: amendments to care about proxy detach and port ID logging
 v3: a minor adjustment in the cover letter subject line
 v4: edits to apply review notes and align with 22.11
 v5: drop the transfer proxy patch; fix CI in [1/2]
 v6: another attempt to fix re-configure in [1/2]

Ivan Malov (2):
  netdev-dpdk: negotiate delivery of per-packet Rx metadata
  netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

 lib/netdev-dpdk.c | 50 +++
 lib/netdev-offload-dpdk.c | 31 +---
 2 files changed, 67 insertions(+), 14 deletions(-)

-- 
2.17.1

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

[ovs-dev] [PATCH v6 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 50 +++
 1 file changed, 50 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8cb1a7703..98765fe6e 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -542,6 +542,9 @@ struct netdev_dpdk {
 int rte_xstats_names_size;
 int rte_xstats_ids_size;
 uint64_t *rte_xstats_ids;
+
+/* Ensures that Rx metadata delivery is configured only once */
+bool rx_metadata_delivery_configured;
 );
 };
 
@@ -1140,6 +1143,41 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+if (dev->rx_metadata_delivery_configured)
+return;
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);
+}
+
+dev->rx_metadata_delivery_configured = true;
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1154,6 +1192,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -1320,6 +1368,8 @@ common_construct(struct netdev *netdev, dpdk_port_t 
port_no,
 /* Initilize the hardware offload flags to 0 */
 dev->hw_ol_features = 0;
 
+dev->rx_metadata_delivery_configured = false;
+
 dev->flags = NETDEV_UP | NETDEV_PROMISC;
 
 ovs_list_push_back(_list, >list_node);
-- 
2.17.1

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

[ovs-dev] [PATCH v5 2/2] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.17.1

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

[ovs-dev] [PATCH v5 0/2] DPDK: align flow offloads with 22.11

[Ivan Malov via dev]

Address two problems using two corresponding features in DPDK,
which have been around for a year and are now stable:

1) The need to make sure that metadata generated by flow
   rule execution be delivered from NIC to application;

2) Replacing PORT_ID action with REPRESENTED_PORT.
---
 v2: amendments to care about proxy detach and port ID logging
 v3: a minor adjustment in the cover letter subject line
 v4: edits to apply review notes and align with 22.11
 v5: drop the transfer proxy patch; fix CI in [1/2]

Ivan Malov (2):
  netdev-dpdk: negotiate delivery of per-packet Rx metadata
  netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

 lib/netdev-dpdk.c | 43 +++
 lib/netdev-offload-dpdk.c | 31 +++-
 2 files changed, 60 insertions(+), 14 deletions(-)

-- 
2.17.1

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

[ovs-dev] [PATCH v5 1/2] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 8cb1a7703..774ca8d51 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1140,6 +1140,39 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else if (ret == -EBUSY) {
+/* Already configured */
+return;
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);
+}
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1154,6 +1187,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
-- 
2.17.1

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

Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism

[Ivan Malov via dev]

Hi Eli,

Thanks for responding. PSB.

On Tue, 6 Jun 2023, Eli Britstein wrote:





-Original Message-
From: Ivan Malov 
Sent: Sunday, 4 June 2023 15:58
To: Eli Britstein 
Cc: ovs-dev@openvswitch.org; Ilya Maximets ; Ori
Kam ; David Marchand 
Subject: RE: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism

External email: Use caution opening links or attachments


Hi Eli,

Thanks for reviewing this. Please see below.

On Tue, 21 Feb 2023, Eli Britstein wrote:





-Original Message-
From: Ivan Malov 
Sent: Tuesday, 21 February 2023 2:41
To: ovs-dev@openvswitch.org
Cc: Ilya Maximets ; Eli Britstein
; Ori Kam ; David Marchand

Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism

External email: Use caution opening links or attachments


Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified explicitly,
via the corresponding pattern item.

Signed-off-by: Ivan Malov 
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h |  4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
2cebc3cca..3a9c9d9a0
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {

struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE,

cacheline0,

+dpdk_port_t flow_transfer_proxy_port_id;

This extra field here makes it overflow one cache line.

dpdk_port_t port_id;

/* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6
+1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa =

RTE_ETH_RX_OFFLOAD_UDP_CKSUM |

 RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
 RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;

/*
 * Full tunnel offload requires that tunnel ID metadata be @@
-1194,6
+1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 */
dpdk_eth_dev_init_rx_metadata(dev);

+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+   >flow_transfer_proxy_port_id, 
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );

if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10
@@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc

OVS_UNUSED,

   const char *argv[], void *aux OVS_UNUSED)  {
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
int argc OVS_UNUSED,
  argv[1]);

RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn

*conn,

int argc OVS_UNUSED,
}
ds_destroy(_interfaces);

+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow

proxy)",

+ argv[1]);

This is not acceptable.
When removing a port, we clean the offloads using

netdev_offload_dpdk_flow_flush().

It should be enhanced to check if the proxy port is detached, remove the

offloads of all the ports that used it.

There is a related patch proposed in [1].
[1]


https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fpatch



work.ozlabs.org%2Fproject%2Fopenvswitch%2Fpatch%2F20220905144603.358
51

05-1-

elibr%40nvidia.com%2F=05%7C01%7Celibr%40nvidia.com%7Ca5083e7



6e913441830f108db64fb5c71%7C43083d15727340c1b7db39efd9ccc17a%7C0%7
C0%7

Re: [ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism

[Ivan Malov via dev]

Hi Eli,

Thanks for reviewing this. Please see below.

On Tue, 21 Feb 2023, Eli Britstein wrote:





-Original Message-
From: Ivan Malov 
Sent: Tuesday, 21 February 2023 2:41
To: ovs-dev@openvswitch.org
Cc: Ilya Maximets ; Eli Britstein ; Ori
Kam ; David Marchand 
Subject: [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy
mechanism

External email: Use caution opening links or attachments


Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified explicitly, via the
corresponding pattern item.

Signed-off-by: Ivan Malov 
---
lib/netdev-dpdk.c | 88 +++
lib/netdev-dpdk.h |  4 +-
lib/netdev-offload-dpdk.c | 55 +++-
3 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 2cebc3cca..3a9c9d9a0
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {

struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;

This extra field here makes it overflow one cache line.

dpdk_port_t port_id;

/* If true, device was attached by rte_eth_dev_attach(). */ @@ -1183,6
+1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
 RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
 RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;

/*
 * Full tunnel offload requires that tunnel ID metadata be @@ -1194,6
+1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 */
dpdk_eth_dev_init_rx_metadata(dev);

+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+   >flow_transfer_proxy_port_id, 
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
rte_eth_dev_info_get(dev->port_id, );

if (strstr(info.driver_name, "vf") != NULL) { @@ -3981,8 +4001,10 @@
netdev_dpdk_detach(struct unixctl_conn *conn, int argc OVS_UNUSED,
   const char *argv[], void *aux OVS_UNUSED)  {
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int
argc OVS_UNUSED,
  argv[1]);

RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn,
int argc OVS_UNUSED,
}
ds_destroy(_interfaces);

+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow 
proxy)",
+ argv[1]);

This is not acceptable.
When removing a port, we clean the offloads using 
netdev_offload_dpdk_flow_flush().
It should be enhanced to check if the proxy port is detached, remove the 
offloads of all the ports that used it.
There is a related patch proposed in [1].
[1] 
http://patchwork.ozlabs.org/project/openvswitch/patch/20220905144603.3585105-1-el...@nvidia.com/


I hear you. But do you want me to first wait for patch [1] to be applied
and then send the proxy enhancement to be applied on top of it?
Or do you believe I can do something in parallel here?

As I can see, patch [1] has not been applied yet.
Is there any problem with it?

If I need to wait, then I believe I can revoke this [3/3]
patch from the series for the time being and send v5.
What's your opinion?




+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy.
*/
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+

Re: [ovs-dev] [PATCH dpdk-latest v4 1/5] netdev-dpdk: use flow transfer proxy

[Ivan Malov via dev]

Hi Nole, Simon, Ilya,

On Mon, 8 May 2023, Nole Zhang wrote:





-Original Message-
From: Ilya Maximets 
Sent: Saturday, May 6, 2023 3:07 AM
To: Ivan Malov ; Simon Horman

Cc: i.maxim...@ovn.org; d...@openvswitch.org; Eli Britstein
; Kevin Liu ; Chaoyong He
; oss-drivers ; Nole
Zhang 
Subject: Re: [ovs-dev] [PATCH dpdk-latest v4 1/5] netdev-dpdk: use flow
transfer proxy

On 5/3/23 16:00, Ivan Malov wrote:

Hello Simon,

This patch has me intrigued. By the looks of it, it bears uncanny
resemblance to patch [1] by another author. Is your patch based on
patch [1]? If yes, could you please comment on the following:

1) Your patch does not seem to reference the original author.
   Why is it so? Is there a problem, colleagues?


When re-using someone else's work, please, retain the original authorship.  I
see there are changes made to the patch, but it's the same as the original in
many parts.  Since you made changes, you should add yourself as co-authors.
If you feel that changes made are more significant than the original ptch,
then you may swap the authorship, but you should add the original author to
the list of co-authors anyway.


Thanks for the clarification Ilya.



@Ivan Malov sorry again.
It is my fault, I'm very sorry for these.
Next version, I will add signed-by.


Glad we see eye to eye about this and can understand
each other after all.





2) Your patch does not seem to address review feedback [2].
   There's a problem that has been indicated by Eli,
   regarding flow flush. Doesn't it still stand?


In this version the rte_flow_flush() call is added instead of failing the 
detach.
However,

a. the flush operation should have already been executed from
   the higher layer from do_del_port() in dpif-netdev.  So,
   it should not be needed.

b. The problem doesn't apper to be addressed, because related
   ports will not get their flows flushed.


Ok, I think you are right, do you think if I use the  rte_flow_flush() for 
related ports is OK?


Please find Eli's review of the original patch where he explains
how it should be done:

https://mail.openvswitch.org/pipermail/ovs-dev/2023-February/402172.html

Thank you.





Best regards, Ilya Maximets.



Interested to hear your input on this. Thank you.

[1]
https://mail.openvswitch.org/pipermail/ovs-dev/2023-February/402152.ht
ml

[2]
https://mail.openvswitch.org/pipermail/ovs-dev/2023-February/402172.ht
ml



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

Re: [ovs-dev] [PATCH dpdk-latest v4 1/5] netdev-dpdk: use flow transfer proxy

[Ivan Malov via dev]

Hello Simon,

This patch has me intrigued. By the looks of it, it bears uncanny
resemblance to patch [1] by another author. Is your patch based
on patch [1]? If yes, could you please comment on the following:

1) Your patch does not seem to reference the original author.
   Why is it so? Is there a problem, colleagues?

2) Your patch does not seem to address review feedback [2].
   There's a problem that has been indicated by Eli,
   regarding flow flush. Doesn't it still stand?

Interested to hear your input on this. Thank you.

[1] 
https://mail.openvswitch.org/pipermail/ovs-dev/2023-February/402152.html


[2] 
https://mail.openvswitch.org/pipermail/ovs-dev/2023-February/402172.html


On Wed, 26 Apr 2023, Simon Horman wrote:


From: Peng Zhang 

Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.

Signed-off-by: Peng Zhang 
Signed-off-by: Jin Liu 
Co-authored-by: Jin Liu 
Signed-off-by: Simon Horman 
---
lib/netdev-dpdk.c | 97 +--
lib/netdev-dpdk.h |  3 ++
lib/netdev-offload-dpdk.c | 16 ---
3 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fff57f78279a..278d6afc08af 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -437,6 +437,7 @@ enum dpdk_hw_ol_features {
struct netdev_dpdk {
PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
dpdk_port_t port_id;
+dpdk_port_t flow_transfer_proxy_port_id;

/* If true, device was attached by rte_eth_dev_attach(). */
bool attached;
@@ -1155,6 +1156,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
 RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
 RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
+
+/* Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+   >flow_transfer_proxy_port_id,
+   NULL);
+if (ret != 0) {
+/* The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}

rte_eth_dev_info_get(dev->port_id, );

@@ -3767,8 +3786,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
   const char *argv[], void *aux OVS_UNUSED)
{
struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct rte_flow_error ferror;
struct rte_eth_dev_info dev_info;
dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
dpdk_port_t port_id;
bool used = false;
char *response;
@@ -3786,8 +3807,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
  argv[1]);

RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
LIST_FOR_EACH (dev, list_node, _list) {
if (dev->port_id != sibling_port_id) {
continue;
@@ -3807,6 +3826,22 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
}
ds_destroy(_interfaces);

+/* The device being detached may happen to be a flow proxy port
+ * for another device (still attached). Update the flow proxy port id.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id != port_id &&
+dev->flow_transfer_proxy_port_id == port_id) {
+if (rte_flow_flush(dev->flow_transfer_proxy_port_id, )) {
+response = xasprintf("Flush port failed, Device '%s' can not"
+ "be detached (flow proxy)", argv[1]);
+goto error;
+}
+break;
+}
+}
+
+
rte_eth_dev_info_get(port_id, _info);
rte_eth_dev_close(port_id);
if (rte_dev_remove(dev_info.device) < 0) {
@@ -3817,6 +3852,16 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
response = xasprintf("All devices shared with device '%s' "
 "have been detached", argv[1]);

+/* The device being detached may happen to be a flow proxy port.
+ * After the flow proxy port was detached, the related ports
+ * will reconfigure the device and update the proxy_port_id.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+ if (dev->flow_transfer_proxy_port_id == port_id) {
+netdev_request_reconfigure(>up);
+}
+}
+
ovs_mutex_unlock(_mutex);
unixctl_command_reply(conn, 

[ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism

[Ivan Malov via dev]

Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 88 +++
 lib/netdev-dpdk.h |  4 +-
 lib/netdev-offload-dpdk.c | 55 +++-
 3 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2cebc3cca..3a9c9d9a0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
 dpdk_port_t port_id;
 
 /* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6 +1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
 
 /*
  * Full tunnel offload requires that tunnel ID metadata be
@@ -1194,6 +1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  */
 dpdk_eth_dev_init_rx_metadata(dev);
 
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+   >flow_transfer_proxy_port_id, 
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -3981,8 +4001,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED)
 {
 struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
 struct rte_eth_dev_info dev_info;
 dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
 dpdk_port_t port_id;
 bool used = false;
 char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
   argv[1]);
 
 RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
 LIST_FOR_EACH (dev, list_node, _list) {
 if (dev->port_id != sibling_port_id) {
 continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }
 ds_destroy(_interfaces);
 
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow 
proxy)",
+ argv[1]);
+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy. */
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+
 rte_eth_dev_info_get(port_id, _info);
 rte_eth_dev_close(port_id);
 if (rte_dev_remove(dev_info.device) < 0) {
@@ -5470,7 +5509,8 @@ unlock:
 }
 
 int
-netdev_dpdk_get_port_id(struct netdev *netdev)
+netdev_dpdk_get_port_id(struct netdev *netdev,
+bool flow_transfer_proxy)
 {
 struct netdev_dpdk *dev;
 int ret = -1;
@@ -5481,7 +5521,7 @@ netdev_dpdk_get_port_id(struct netdev *netdev)
 
 dev = netdev_dpdk_cast(netdev);
 ovs_mutex_lock(>mutex);
-ret = dev->port_id;
+ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id : 
dev->port_id;
 ovs_mutex_unlock(>mutex);
 out:
 return ret;
@@ -5517,13 +5557,15 @@ out:
 
 int
 netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
- struct rte_flow *rte_flow,
+ bool transfer, struct rte_flow *rte_flow,
  struct rte_flow_error *error)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 int ret;
 
-ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+ret = rte_flow_destroy(transfer ?
+   dev->flow_transfer_proxy_port_id : dev->port_id,
+   rte_flow, 

[ovs-dev] [PATCH v4 0/3] DPDK: align flow offloads with 22.11

[Ivan Malov via dev]

Address three problems using three corresponding features in
DPDK, which have been around for a year and are now stable:

1) The need to make sure that metadata generated by flow
   rule execution be delivered from NIC to application;

2) Replacing PORT_ID action with REPRESENTED_PORT;

3) Potential need to manage "transfer" flows
   via a privileged ("transfer proxy") port.
---
 v2: amendments to care about proxy detach and port ID logging
 v3: a minor adjustment in the cover letter subject line
 v4: edits to apply review notes and align with 22.11

Ivan Malov (3):
  netdev-dpdk: negotiate delivery of per-packet Rx metadata
  netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT
  netdev-offload-dpdk: use flow transfer proxy mechanism

 lib/netdev-dpdk.c | 128 +-
 lib/netdev-dpdk.h |   4 +-
 lib/netdev-offload-dpdk.c |  86 +
 3 files changed, 174 insertions(+), 44 deletions(-)

-- 
2.20.1

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

[ovs-dev] [PATCH v4 1/3] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fb0dd43f7..2cebc3cca 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1140,6 +1140,36 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);
+}
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1154,6 +1184,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
-- 
2.20.1

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

[ovs-dev] [PATCH v4 2/3] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 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 b3421c099..7f2598a53 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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.20.1

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

[ovs-dev] [PATCH v4 2/3] netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT

[Ivan Malov via dev]

Action PORT_ID has been deprecated. Use REPRESENTED_PORT instead.

Signed-off-by: Ivan Malov 
---
 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 b3421c099..7f2598a53 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(, "%s: Output to port \'%s\' cannot be offloaded.",
 netdev_get_name(netdev), netdev_get_name(outdev));
 ret = -1;
-- 
2.20.1

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

[ovs-dev] [PATCH v4 1/3] netdev-dpdk: negotiate delivery of per-packet Rx metadata

[Ivan Malov via dev]

This may be required by some PMDs in offload scenarios.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index fb0dd43f7..2cebc3cca 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1140,6 +1140,36 @@ dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) 
OVS_REQUIRES(dev->mutex)
 }
 }
 
+static void
+dpdk_eth_dev_init_rx_metadata(struct netdev_dpdk *dev)
+{
+uint64_t rx_metadata = 0;
+int ret;
+
+/* For the fallback offload (non-"transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_USER_MARK;
+/* For the full offload ("transfer" rules) */
+rx_metadata |= RTE_ETH_RX_METADATA_TUNNEL_ID;
+
+ret = rte_eth_rx_metadata_negotiate(dev->port_id, _metadata);
+if (ret == 0) {
+if (!(rx_metadata & RTE_ETH_RX_METADATA_USER_MARK)) {
+VLOG_DBG("The NIC will not provide per-packet USER_MARK on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+if (!(rx_metadata & RTE_ETH_RX_METADATA_TUNNEL_ID)) {
+VLOG_DBG("The NIC will not provide per-packet TUNNEL_ID on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+}
+} else if (ret == -ENOTSUP) {
+VLOG_DBG("Rx metadata negotiate procedure is not supported on port "
+ DPDK_PORT_ID_FMT, dev->port_id);
+} else {
+VLOG_WARN("Cannot negotiate Rx metadata on port "
+  DPDK_PORT_ID_FMT, dev->port_id);
+}
+}
+
 static int
 dpdk_eth_dev_init(struct netdev_dpdk *dev)
 OVS_REQUIRES(dev->mutex)
@@ -1154,6 +1184,16 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
 
+/*
+ * Full tunnel offload requires that tunnel ID metadata be
+ * delivered with "miss" packets from the hardware to the
+ * PMD. The same goes for megaflow mark metadata which is
+ * used in MARK + RSS offload scenario.
+ *
+ * Request delivery of such metadata.
+ */
+dpdk_eth_dev_init_rx_metadata(dev);
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
-- 
2.20.1

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

[ovs-dev] [PATCH v4 3/3] netdev-offload-dpdk: use flow transfer proxy mechanism

[Ivan Malov via dev]

Manage "transfer" flows via the corresponding mechanism.
Doing so requires that the traffic source be specified
explicitly, via the corresponding pattern item.

Signed-off-by: Ivan Malov 
---
 lib/netdev-dpdk.c | 88 +++
 lib/netdev-dpdk.h |  4 +-
 lib/netdev-offload-dpdk.c | 55 +++-
 3 files changed, 117 insertions(+), 30 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 2cebc3cca..3a9c9d9a0 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -434,6 +434,7 @@ enum dpdk_hw_ol_features {
 
 struct netdev_dpdk {
 PADDED_MEMBERS_CACHELINE_MARKER(CACHE_LINE_SIZE, cacheline0,
+dpdk_port_t flow_transfer_proxy_port_id;
 dpdk_port_t port_id;
 
 /* If true, device was attached by rte_eth_dev_attach(). */
@@ -1183,6 +1184,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
 uint32_t rx_chksm_offload_capa = RTE_ETH_RX_OFFLOAD_UDP_CKSUM |
  RTE_ETH_RX_OFFLOAD_TCP_CKSUM |
  RTE_ETH_RX_OFFLOAD_IPV4_CKSUM;
+int ret;
 
 /*
  * Full tunnel offload requires that tunnel ID metadata be
@@ -1194,6 +1196,24 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
  */
 dpdk_eth_dev_init_rx_metadata(dev);
 
+/*
+ * Managing "transfer" flows requires that the user communicate them
+ * via a port which has the privilege to control the embedded switch.
+ * For some vendors, all ports in a given switching domain have
+ * this privilege. For other vendors, it's only one port.
+ *
+ * Get the proxy port ID and remember it for later use.
+ */
+ret = rte_flow_pick_transfer_proxy(dev->port_id,
+   >flow_transfer_proxy_port_id, 
NULL);
+if (ret != 0) {
+/*
+ * The PMD does not indicate the proxy port.
+ * Assume the proxy is unneeded.
+ */
+dev->flow_transfer_proxy_port_id = dev->port_id;
+}
+
 rte_eth_dev_info_get(dev->port_id, );
 
 if (strstr(info.driver_name, "vf") != NULL) {
@@ -3981,8 +4001,10 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
const char *argv[], void *aux OVS_UNUSED)
 {
 struct ds used_interfaces = DS_EMPTY_INITIALIZER;
+struct netdev_dpdk *dev_self = NULL;
 struct rte_eth_dev_info dev_info;
 dpdk_port_t sibling_port_id;
+struct netdev_dpdk *dev;
 dpdk_port_t port_id;
 bool used = false;
 char *response;
@@ -4000,8 +4022,6 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
   argv[1]);
 
 RTE_ETH_FOREACH_DEV_SIBLING (sibling_port_id, port_id) {
-struct netdev_dpdk *dev;
-
 LIST_FOR_EACH (dev, list_node, _list) {
 if (dev->port_id != sibling_port_id) {
 continue;
@@ -4021,6 +4041,25 @@ netdev_dpdk_detach(struct unixctl_conn *conn, int argc 
OVS_UNUSED,
 }
 ds_destroy(_interfaces);
 
+/*
+ * The device being detached may happen to be a flow proxy port
+ * for another device (still attached). If so, do not allow to
+ * detach. Devices dependent on this one must be detached first.
+ */
+LIST_FOR_EACH (dev, list_node, _list) {
+if (dev->port_id == port_id) {
+dev_self = dev;
+} else if (dev->flow_transfer_proxy_port_id == port_id) {
+response = xasprintf("Device '%s' can not be detached (flow 
proxy)",
+ argv[1]);
+goto error;
+}
+}
+
+/* Indicate that the device being detached no longer needs a flow proxy. */
+if (dev_self != NULL)
+dev_self->flow_transfer_proxy_port_id = port_id;
+
 rte_eth_dev_info_get(port_id, _info);
 rte_eth_dev_close(port_id);
 if (rte_dev_remove(dev_info.device) < 0) {
@@ -5470,7 +5509,8 @@ unlock:
 }
 
 int
-netdev_dpdk_get_port_id(struct netdev *netdev)
+netdev_dpdk_get_port_id(struct netdev *netdev,
+bool flow_transfer_proxy)
 {
 struct netdev_dpdk *dev;
 int ret = -1;
@@ -5481,7 +5521,7 @@ netdev_dpdk_get_port_id(struct netdev *netdev)
 
 dev = netdev_dpdk_cast(netdev);
 ovs_mutex_lock(>mutex);
-ret = dev->port_id;
+ret = flow_transfer_proxy ? dev->flow_transfer_proxy_port_id : 
dev->port_id;
 ovs_mutex_unlock(>mutex);
 out:
 return ret;
@@ -5517,13 +5557,15 @@ out:
 
 int
 netdev_dpdk_rte_flow_destroy(struct netdev *netdev,
- struct rte_flow *rte_flow,
+ bool transfer, struct rte_flow *rte_flow,
  struct rte_flow_error *error)
 {
 struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
 int ret;
 
-ret = rte_flow_destroy(dev->port_id, rte_flow, error);
+ret = rte_flow_destroy(transfer ?
+   dev->flow_transfer_proxy_port_id : dev->port_id,
+   rte_flow, 

[ovs-dev] [PATCH v4 0/3] DPDK: align flow offloads with 22.11

[Ivan Malov via dev]

Address three problems using three corresponding features in
DPDK, which have been around for a year and are now stable:

1) The need to make sure that metadata generated by flow
   rule execution be delivered from NIC to application;

2) Replacing PORT_ID action with REPRESENTED_PORT;

3) Potential need to manage "transfer" flows
   via a privileged ("transfer proxy") port.
---
 v2: amendments to care about proxy detach and port ID logging
 v3: a minor adjustment in the cover letter subject line
 v4: edits to apply review notes and align with 22.11

Ivan Malov (3):
  netdev-dpdk: negotiate delivery of per-packet Rx metadata
  netdev-offload-dpdk: replace action PORT_ID with REPRESENTED_PORT
  netdev-offload-dpdk: use flow transfer proxy mechanism

 lib/netdev-dpdk.c | 128 +-
 lib/netdev-dpdk.h |   4 +-
 lib/netdev-offload-dpdk.c |  86 +
 3 files changed, 174 insertions(+), 44 deletions(-)

-- 
2.20.1

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